diff --git a/news/6457.feature.md b/news/6457.feature.md new file mode 100644 index 00000000000..cdaad1f5e64 --- /dev/null +++ b/news/6457.feature.md @@ -0,0 +1 @@ +Auto-memoized (`rx.memo`) components now compile to `.web/app_components/` output paths that mirror their defining Python source module (using the real package name, including framework packages) instead of being bundled into a single shared `components.jsx`. The compiler's auto-memo registry is scoped per source module, so identical-rendering subtrees in different modules each emit their own output instead of one silently overwriting another, hot-reloads of a module refresh the correct output, and stale memo files are cleaned up when their source changes. Memos whose module can't be mirrored (`__main__`, unsafe names) fall back to one file per memo at `.web/utils/components/.jsx`. Each mirrored memo's generated export name also carries a stable per-module suffix, so two memos that share a name in different modules compile to distinct symbols and can be used together on one page without colliding. diff --git a/packages/reflex-base/news/6457.feature.md b/packages/reflex-base/news/6457.feature.md new file mode 100644 index 00000000000..9d0e5dc49c0 --- /dev/null +++ b/packages/reflex-base/news/6457.feature.md @@ -0,0 +1 @@ +Added `reflex_base.utils.memo_paths`, which translates a memo's Python source module into the mirrored `.web/app_components/` JSX path and `$/...` library specifier used by the compiler. The memo component and compiler plugin now route each memo's compiled output through these helpers so it lands alongside its source module's layout, falling back to the per-name `utils/components/` path when the module can't be mirrored. The helpers also derive a per-module-unique JS symbol for each mirrored memo, and the memo registry is keyed by `(name, source module)` so same-named memos defined in different modules coexist instead of colliding. diff --git a/packages/reflex-base/src/reflex_base/compiler/templates.py b/packages/reflex-base/src/reflex_base/compiler/templates.py index 0d2f750e9eb..a380aaac4b9 100644 --- a/packages/reflex-base/src/reflex_base/compiler/templates.py +++ b/packages/reflex-base/src/reflex_base/compiler/templates.py @@ -8,6 +8,7 @@ from reflex_base import constants from reflex_base.constants import Hooks +from reflex_base.utils import memo_paths from reflex_base.utils.format import format_state_name, json_dumps from reflex_base.vars.base import VarData @@ -192,7 +193,7 @@ def app_root_template( if hydrate_fallback_export is not None: hydrate_fallback_str = ( f"export {{ {hydrate_fallback_export} as HydrateFallback }} " - f'from "$/{constants.Dirs.COMPONENTS_PATH}/{hydrate_fallback_export}";' + f'from "{memo_paths.unmirrored_library_specifier(hydrate_fallback_export)}";' ) custom_code_str = "\n".join(custom_codes) diff --git a/packages/reflex-base/src/reflex_base/components/memo.py b/packages/reflex-base/src/reflex_base/components/memo.py index c31e37dd9db..979d1501bd5 100644 --- a/packages/reflex-base/src/reflex_base/components/memo.py +++ b/packages/reflex-base/src/reflex_base/components/memo.py @@ -39,7 +39,7 @@ ) from reflex_base.constants.state import CAMEL_CASE_MEMO_MARKER from reflex_base.event import EventChain, EventHandler, no_args_event_spec, run_script -from reflex_base.utils import console, format +from reflex_base.utils import console, format, memo_paths from reflex_base.utils.imports import ImportVar from reflex_base.utils.types import safe_issubclass, typehint_issubclass from reflex_base.vars import VarData @@ -253,6 +253,12 @@ class MemoDefinition: fn: Callable[..., Any] python_name: str params: tuple[MemoParam, ...] + # The Python module that defined this memo. When set, the memo's compiled + # JSX is emitted to a path mirroring that module and the page-side import + # resolves there instead of the per-name ``utils/components/`` path + # used for memos that can't be mirrored. ``kw_only`` so subclasses can keep + # their own required fields. + source_module: str | None = dataclasses.field(default=None, kw_only=True) @dataclasses.dataclass(frozen=True, slots=True) @@ -346,6 +352,7 @@ def _post_init(self, **kwargs): def _get_memo_component_class( export_name: str, wrapped_component_type: type[Component] = Component, + source_module: str | None = None, ) -> type[MemoComponent]: """Get the component subclass for a memo export. @@ -360,18 +367,21 @@ def _get_memo_component_class( wrapped_component_type: The class of the component being memoized. Defaults to ``Component`` for memos that don't wrap a user component (e.g. function memos, raw passthroughs). + source_module: The user-app Python module that defined this memo. When + set, the wrapper imports from a path mirroring that module instead + of the per-name ``utils/components/`` path. Returns: A cached component subclass with the tag set at class definition time. """ + # With a source module the memo is grouped into a file mirroring its + # Python module; otherwise each memo gets its own per-file module so Vite + # has distinct module boundaries per memo, enabling code-split by page. + library, symbol = memo_paths.library_and_symbol(source_module, export_name) attrs: dict[str, Any] = { "__module__": __name__, - "tag": export_name, - # Point each memo at its own per-file module so pages import directly - # from ``$/utils/components/`` rather than through the index. - # Per-file import paths give Vite distinct module boundaries per - # memo, enabling actual code-split by page. - "library": f"$/{constants.Dirs.COMPONENTS_PATH}/{export_name}", + "tag": symbol, + "library": library, "_wrapped_component_type": wrapped_component_type, } if ( @@ -382,27 +392,43 @@ def _get_memo_component_class( wrapped_component_type._get_app_wrap_components ) return type( - f"MemoComponent_{export_name}", + f"MemoComponent_{symbol}", (MemoComponent,), attrs, ) -MEMOS: dict[str, MemoDefinition] = {} +def reset_memo_component_classes() -> None: + """Clear the cached memo wrapper classes. + Called at the start of each compile so a memo's ``library`` is recomputed + from the current module layout. Without this, a module that switches to a + package (or back) between hot-reload compiles would keep serving the + library specifier resolved on the first compile, pointing pages at an + output path the compiler no longer writes. + """ + _get_memo_component_class.cache_clear() + + +MEMOS: dict[tuple[str, str | None], MemoDefinition] = {} -def _memo_registry_key(definition: MemoDefinition) -> str: + +def _memo_registry_key(definition: MemoDefinition) -> tuple[str, str | None]: """Get the registry key for a memo. + The key pairs the compiled name with the source module: two memos with the + same name in different modules compile to distinct files (and distinct JS + symbols), so they must register as separate entries rather than colliding. + Args: definition: The memo definition. Returns: - The registry key for the memo. + The ``(name, source_module)`` registry key for the memo. """ if isinstance(definition, MemoComponentDefinition): - return definition.export_name - return definition.python_name + return definition.export_name, definition.source_module + return definition.python_name, definition.source_module def _is_memo_reregistration( @@ -440,10 +466,10 @@ def _register_memo_definition(definition: MemoDefinition) -> None: not _is_memo_reregistration(existing, definition) ): msg = ( - f"Memo name collision for `{key}`: " + f"Memo name collision for `{key[0]}`: " f"`{existing.fn.__module__}.{existing.python_name}` and " f"`{definition.fn.__module__}.{definition.python_name}` both compile " - "to the same memo name." + "to the same memo name in the same module." ) raise ValueError(msg) @@ -627,42 +653,48 @@ def _get_rest_param(params: tuple[MemoParam, ...]) -> MemoParam | None: return next((p for p in params if p.kind is MemoParamKind.REST), None) -def _imported_function_var(name: str, return_type: Any) -> FunctionVar: +def _imported_function_var( + name: str, return_type: Any, source_module: str | None = None +) -> FunctionVar: """Create the imported FunctionVar for a memo. Args: name: The exported function name. return_type: The return type of the function. + source_module: The Python module that defined the memo. When set, the + import resolves to the mirrored module file instead of the per-name + ``utils/components/`` path. Returns: The imported FunctionVar. """ + library, symbol = memo_paths.library_and_symbol(source_module, name) return FunctionStringVar.create( - name, + symbol, _var_type=ReflexCallable[Any, return_type], - _var_data=VarData( - imports={ - f"$/{constants.Dirs.COMPONENTS_PATH}/{name}": [ImportVar(tag=name)] - } - ), + _var_data=VarData(imports={library: [ImportVar(tag=symbol)]}), ) -def _component_import_var(name: str) -> Var: +def _component_import_var(name: str, source_module: str | None = None) -> Var: """Create the imported component var for a memo component. Args: name: The exported component name. + source_module: The Python module that defined the memo. When set, the + import resolves to the mirrored module file instead of the per-name + ``utils/components/`` path. Returns: The component var. """ + library, symbol = memo_paths.library_and_symbol(source_module, name) return Var( - name, + symbol, _var_type=type[Component], _var_data=VarData( imports={ - f"$/{constants.Dirs.COMPONENTS_PATH}/{name}": [ImportVar(tag=name)], + library: [ImportVar(tag=symbol)], "@emotion/react": [ImportVar(tag="jsx")], } ), @@ -1311,12 +1343,14 @@ def _evaluate_function_body( def _create_component_definition( fn: Callable[..., Any], return_annotation: Any, + source_module: str | None = None, ) -> MemoComponentDefinition: """Create a definition for a component-returning memo. Args: fn: The function to analyze. return_annotation: The return annotation. + source_module: The user-app Python module that defined the memo. Returns: The component memo definition. @@ -1329,6 +1363,7 @@ def _create_component_definition( fn=fn, python_name=fn.__name__, params=params, + source_module=source_module, export_name=format.to_title_case(fn.__name__), _component=_LazyBody.ready(_evaluate_component_body(fn, params)), ) @@ -1594,7 +1629,9 @@ def __call__(self, *children: Any, **props: Any) -> MemoComponent: # Reading ``component`` materializes the deferred body, so ``type(...)`` # reflects the real wrapped class rather than the placeholder. return _get_memo_component_class( - definition.export_name, type(definition.component) + definition.export_name, + type(definition.component), + definition.source_module, )._create( children=list(children), memo_definition=definition, @@ -1608,7 +1645,9 @@ def _as_var(self) -> Var: Returns: The imported component var. """ - return _component_import_var(self._definition.export_name) + return _component_import_var( + self._definition.export_name, self._definition.source_module + ) def _create_function_wrapper( @@ -1641,6 +1680,7 @@ def _create_component_wrapper( def create_passthrough_component_memo( component: Component, + source_module: str | None = None, ) -> tuple[ Callable[..., MemoComponent], MemoComponentDefinition, @@ -1659,6 +1699,8 @@ def create_passthrough_component_memo( Args: component: The component to wrap. + source_module: The user-app Python module that triggered creation of + this memo (typically the page that contained the wrapped subtree). Returns: The callable memo wrapper and its component definition. @@ -1726,7 +1768,7 @@ def passthrough(children: Var[Component]) -> Component: passthrough.__qualname__ = passthrough.__name__ passthrough.__module__ = __name__ - definition = _create_component_definition(passthrough, Component) + definition = _create_component_definition(passthrough, Component, source_module) replacements: dict[str, Any] = {} if definition.export_name != tag: replacements["export_name"] = tag @@ -1871,6 +1913,8 @@ def memo(fn: Callable[..., Any]) -> _MemoComponentWrapper | _MemoFunctionWrapper defaulted_params=defaulted_params, ) + source_module = memo_paths.capture_source_module(fn) + if missing_return or defaulted_params: _warn_missing_annotations(fn.__name__, missing_return, defaulted_params) @@ -1885,6 +1929,7 @@ def memo(fn: Callable[..., Any]) -> _MemoComponentWrapper | _MemoFunctionWrapper fn=fn, python_name=fn.__name__, params=params, + source_module=source_module, export_name=format.to_title_case(fn.__name__), _component=_LazyBody( lambda: _evaluate_component_body(fn, params), @@ -1897,9 +1942,12 @@ def memo(fn: Callable[..., Any]) -> _MemoComponentWrapper | _MemoFunctionWrapper fn=fn, python_name=fn.__name__, params=params, + source_module=source_module, _function=_LazyBody(lambda: _evaluate_function_body(fn, params)), imported_var=_imported_function_var( - fn.__name__, _annotation_inner_type(return_annotation) + fn.__name__, + _annotation_inner_type(return_annotation), + source_module=source_module, ), ) wrapper = _create_function_wrapper(definition) diff --git a/packages/reflex-base/src/reflex_base/constants/base.py b/packages/reflex-base/src/reflex_base/constants/base.py index 620d9bd7ee0..dc06fbf0cb9 100644 --- a/packages/reflex-base/src/reflex_base/constants/base.py +++ b/packages/reflex-base/src/reflex_base/constants/base.py @@ -32,7 +32,8 @@ class Dirs(SimpleNamespace): UTILS = "utils" # The name of the state file. STATE_PATH = UTILS + "/state" - # The name of the components file. + # The name of the components file, where memos that can't be mirrored to a + # user module (``__main__``, unsafe names) get one file per memo. COMPONENTS_PATH = UTILS + "/components" # The name of the contexts file. CONTEXTS_PATH = UTILS + "/context" @@ -48,6 +49,10 @@ class Dirs(SimpleNamespace): PAGES = "app" # The name of the routes directory. ROUTES = "routes" + # Subdirectory holding memo modules mirrored from their defining Python + # module, kept separate from other ``.web`` output so a mirrored module + # path can't collide with framework files (e.g. ``app/``, ``utils/``). + APP_COMPONENTS = "app_components" # The name of the env json file. ENV_JSON = "env.json" # The name of the reflex json file. diff --git a/packages/reflex-base/src/reflex_base/plugins/compiler.py b/packages/reflex-base/src/reflex_base/plugins/compiler.py index 5720d27aab7..c8ffe6d2711 100644 --- a/packages/reflex-base/src/reflex_base/plugins/compiler.py +++ b/packages/reflex-base/src/reflex_base/plugins/compiler.py @@ -689,6 +689,7 @@ class PageContext(BaseContext): frontend_imports: ParsedImportDict = dataclasses.field(default_factory=dict) output_path: str | None = None output_code: str | None = None + source_module: str | None = None # Stack of ``id(component)`` for components whose subtree is # memoize-suppressed. Populated by ``MemoizeStatefulPlugin`` when it # encounters a ``MemoizationLeaf``-style snapshot boundary and popped on @@ -766,8 +767,13 @@ class CompileContext(BaseContext): # ``MemoizeStatefulPlugin``). memoize_wrappers: dict[str, None] = dataclasses.field(default_factory=dict) # Compiler-generated memo definitions for auto-memoized stateful wrappers. - # Stored as ``Any`` to avoid an import cycle with ``reflex_base.components.memo``. - auto_memo_components: dict[str, Any] = dataclasses.field(default_factory=dict) + # Keyed by ``(tag, source_module)`` so identical-rendering subtrees from + # different user modules each get their own entry and emit into the right + # mirrored memo file. Stored as ``Any`` to avoid an import cycle with + # ``reflex_base.components.memo``. + auto_memo_components: dict[tuple[str, str | None], Any] = dataclasses.field( + default_factory=dict + ) def compile( self, diff --git a/packages/reflex-base/src/reflex_base/utils/imports.py b/packages/reflex-base/src/reflex_base/utils/imports.py index e4ec1935739..7e614f99524 100644 --- a/packages/reflex-base/src/reflex_base/utils/imports.py +++ b/packages/reflex-base/src/reflex_base/utils/imports.py @@ -6,6 +6,16 @@ from collections import defaultdict from collections.abc import Mapping, Sequence +# Absolute import paths beginning with one of these reserved ``.web`` +# subdirectories are rewritten to ``$``-prefixed module specifiers. +ABSOLUTE_IMPORT_PREFIXES = ( + "/utils/", + "/components/", + "/styles/", + "/public/", + "/app_components/", +) + def merge_parsed_imports( *imports: ImmutableParsedImportDict, @@ -42,11 +52,7 @@ def merge_imports( import_dict if isinstance(import_dict, tuple) else import_dict.items() ): # If the lib is an absolute path, we need to prefix it with a $ - lib = ( - "$" + lib - if lib.startswith(("/utils/", "/components/", "/styles/", "/public/")) - else lib - ) + lib = "$" + lib if lib.startswith(ABSOLUTE_IMPORT_PREFIXES) else lib if isinstance(fields, (list, tuple, set)): all_imports[lib].extend( ImportVar(field) if isinstance(field, str) else field diff --git a/packages/reflex-base/src/reflex_base/utils/memo_paths.py b/packages/reflex-base/src/reflex_base/utils/memo_paths.py new file mode 100644 index 00000000000..6ab58c59b33 --- /dev/null +++ b/packages/reflex-base/src/reflex_base/utils/memo_paths.py @@ -0,0 +1,317 @@ +"""Mirror user-app Python module paths into the compiler's ``.web`` output. + +The compiler uses these helpers to write each memo's compiled JSX to a path +under ``.web/app_components/`` that mirrors its Python source module, instead of +bundling everything into one file. This module owns the small set of helpers +that: + +- Read ``fn.__module__``, rejecting only synthetic modules (``__main__``, + missing). Framework modules mirror to their real package name like any other. +- Walk the live frame stack as a fallback for entry points that don't take a + user-supplied callable (notably ``app.add_page(component)`` with a Component + instance), skipping framework frames to reach the real user module. +- Translate a dotted Python module name into mirrored JSX path segments and + the corresponding ``$/...`` library specifier consumed by the import system. +""" + +from __future__ import annotations + +import hashlib +import importlib.util +import inspect +import sys +from collections.abc import Callable +from pathlib import Path + +from reflex_base.constants.base import Dirs + +# Framework packages, matched by exact name or dotted-prefix, used only to steer +# the frame-walk fallback (:func:`resolve_user_module_from_frame`) past framework +# frames to the real user module. Memos defined in these packages still mirror to +# their real package name like any other module. +# +# Matched by exact name plus a dot boundary rather than a bare prefix so a +# developer's own package can't be mistaken for the framework — notably the +# ``reflex_components_*`` family is *not* listed: that's the convention community +# component libraries follow, none of them wrap ``add_page`` (the only thing the +# frame walk skips), and if one did we'd want its memos mirrored to its real +# package name anyway. +_FRAMEWORK_PACKAGES = ( + "reflex", + "reflex_base", + "reflex_site_shared", + "reflex_hosting_cli", + "reflex_docgen", +) + + +def _is_framework_module(module_name: str) -> bool: + """Whether ``module_name`` belongs to the framework itself. + + Used only by :func:`resolve_user_module_from_frame` to skip framework + frames; framework modules are otherwise mirrored like any user module. + + Args: + module_name: The dotted module name. + + Returns: + True if the module is part of the framework. + """ + return any( + module_name == pkg or module_name.startswith(pkg + ".") + for pkg in _FRAMEWORK_PACKAGES + ) + + +def capture_source_module(fn: Callable | None) -> str | None: + """Return the user-app module name for ``fn``, or ``None`` if not user code. + + Reads ``fn.__module__`` directly — Python sets this on every function + definition, and it survives re-exports, decorators that ``functools.wraps`` + correctly, and aliasing. Returns ``None`` only for ``__main__`` and missing + modules, which have no stable path to mirror; framework modules mirror to + their real package name like any other. + + Args: + fn: The callable whose definition module is wanted. + + Returns: + The dotted module name to mirror under ``.web/app_components/``, or + ``None`` to fall back to the per-name un-mirrored output path. + """ + if fn is None: + return None + module_name = getattr(fn, "__module__", None) + if not module_name or module_name == "__main__": + return None + return module_name + + +def resolve_user_module_from_frame(skip: int = 0) -> str | None: + """Walk the live frame stack and return the first user-app module name. + + Used only as a fallback for ``app.add_page(component)`` when the caller + passed a pre-built ``Component`` instance instead of a callable, so there + is no ``__module__`` to read directly. + + Args: + skip: Number of frames above the immediate caller to skip before + starting the search. Pass ``1`` to ignore the function that is + calling this helper. + + Returns: + The first frame's module name that is not a framework module, or + ``None`` if no suitable frame exists (e.g. running inside a REPL). + """ + frame = inspect.currentframe() + if frame is None: + return None + frame = frame.f_back + for _ in range(skip): + if frame is None: + return None + frame = frame.f_back + while frame is not None: + module_name = frame.f_globals.get("__name__") + if ( + module_name + and module_name != "__main__" + and not _is_framework_module(module_name) + ): + return module_name + frame = frame.f_back + return None + + +# Reserved device names on Windows. A file named like one of these (in any +# case, with or without an extension) can't be created normally, so modules +# with such a segment fall back to the un-mirrored output path. +_WINDOWS_RESERVED_NAMES = frozenset({ + "con", + "prn", + "aux", + "nul", + *(f"com{i}" for i in range(1, 10)), + *(f"lpt{i}" for i in range(1, 10)), +}) + + +def _segment_is_safe(segment: str) -> bool: + """Whether ``segment`` is a path-safe Python identifier-like fragment. + + Args: + segment: A single dotted-module segment. + + Returns: + True if the segment can be used as a directory or filename without + introducing path traversal or platform-specific quirks. + """ + if not segment or segment in {".", ".."}: + return False + if any(ch in segment for ch in ("/", "\\", ":", "\0")): + return False + # Windows silently strips trailing dots/spaces and reserves device names, + # either of which breaks the module<->file path correspondence there. + if segment != segment.rstrip(". "): + return False + return segment.casefold() not in _WINDOWS_RESERVED_NAMES + + +def module_to_mirrored_segments(module_name: str | None) -> tuple[str, ...] | None: + """Translate a dotted module name to a tuple of mirrored path segments. + + For a *package* (a module whose import resolves to ``__init__.py``), an + extra ``"index"`` segment is appended so the file lives at + ``/index.jsx`` and submodule files can coexist alongside it as + siblings under ``/``. + + Args: + module_name: The dotted Python module name. ``None`` short-circuits. + + Returns: + A tuple of safe path segments to join under ``.web/app_components/``, or + ``None`` if the module name is missing, contains unsafe segments, or + cannot be resolved as a package vs. module. + """ + if not module_name: + return None + segments = module_name.split(".") + if not all(_segment_is_safe(seg) for seg in segments): + return None + # Prefer the live module's __file__ over a fresh find_spec lookup. A user + # can switch a module to a package (or back) between hot-reload compiles, + # and importlib re-binds __file__ when that happens — a cached find_spec + # result wouldn't. + origin: str | None = None + module = sys.modules.get(module_name) + if module is not None: + origin = getattr(module, "__file__", None) + if origin is None: + try: + spec = importlib.util.find_spec(module_name) + except (ImportError, ValueError): + spec = None + if spec is not None: + origin = spec.origin + if origin and origin.endswith("__init__.py"): + return (*segments, "index") + return tuple(segments) + + +def mirrored_jsx_path(web_dir: Path, segments: tuple[str, ...]) -> Path: + """Build the absolute ``.jsx`` path under ``web_dir`` for ``segments``. + + Mirrored memos live under the reserved ``app_components/`` subdirectory so a + user module path can never collide with framework output (e.g. a memo in + module ``app.root`` would otherwise overwrite ``.web/app/root.jsx``). + + Args: + web_dir: The project's ``.web`` directory. + segments: Mirrored path segments from + :func:`module_to_mirrored_segments`. + + Returns: + The absolute path the compiler should write the memo module to. + """ + return web_dir.joinpath(Dirs.APP_COMPONENTS, *segments).with_suffix(".jsx") + + +def mirrored_library_specifier(segments: tuple[str, ...]) -> str: + """Build the ``$/...`` import specifier for mirrored ``segments``. + + The specifier has no extension; Vite resolves the ``.jsx`` automatically. It + mirrors :func:`mirrored_jsx_path`, including the reserved ``app_components/`` + subdirectory. + + Args: + segments: Mirrored path segments from + :func:`module_to_mirrored_segments`. + + Returns: + A ``$/`` prefixed module specifier suitable for use as a + ``Component.library`` value. + """ + return "$/" + "/".join((Dirs.APP_COMPONENTS, *segments)) + + +def unmirrored_library_specifier(name: str) -> str: + """Build the ``$/...`` import specifier for an un-mirrorable memo. + + Memos that can't be mirrored to a source module (``__main__``, unsafe + names) get one file per memo under ``utils/components/`` — the legacy + per-name layout. + + Args: + name: The memo's export name. + + Returns: + A ``$/`` prefixed module specifier suitable for use as a + ``Component.library`` value. + """ + return f"$/{Dirs.COMPONENTS_PATH}/{name}" + + +def library_for(source_module: str | None, name: str) -> str: + """Return the library specifier a memo should import from. + + Mirrors ``source_module`` when it can be safely mirrored, otherwise falls + back to the per-name ``utils/components/`` path. + + Args: + source_module: The dotted module name a memo was defined in. + name: The memo's export name, used for the un-mirrored fallback. + + Returns: + A ``$/`` prefixed module specifier for the memo's compiled output. + """ + if source_module is not None: + segments = module_to_mirrored_segments(source_module) + if segments is not None: + return mirrored_library_specifier(segments) + return unmirrored_library_specifier(name) + + +def mirrored_symbol(name: str, source_module: str) -> str: + """Return the unique JS symbol a mirrored memo exports and imports under. + + Two memos with the same ``name`` in different modules must compile to + different JS symbols, or a page importing both hits a tag collision. The + output file already mirrors ``source_module``; this gives the symbol the + same per-module uniqueness by appending a short hash of the full dotted + module name. The hash is taken over the dotted name rather than the mirrored + segments so it stays injective: ``a.b`` and ``a_b`` collapse to the same + segments but hash differently. + + Args: + name: The memo's clean export name (e.g. ``MyWidget``). + source_module: The dotted Python module the memo was defined in. + + Returns: + A valid JS identifier unique to ``(name, source_module)``. + """ + suffix = hashlib.sha1(source_module.encode()).hexdigest()[:8] + return f"{name}_{suffix}" + + +def library_and_symbol(source_module: str | None, name: str) -> tuple[str, str]: + """Return the library specifier and JS symbol a memo compiles to. + + Mirrors :func:`library_for` but also returns the symbol, computing the + mirrored segments once so the library and the symbol can never disagree + about whether the module is mirrorable. + + Args: + source_module: The dotted module name a memo was defined in. + name: The memo's export name. + + Returns: + A ``(library, symbol)`` pair. For an un-mirrorable memo the symbol is + ``name`` unchanged and the library is the per-name fallback path. + """ + if source_module is not None: + segments = module_to_mirrored_segments(source_module) + if segments is not None: + return mirrored_library_specifier(segments), mirrored_symbol( + name, source_module + ) + return unmirrored_library_specifier(name), name diff --git a/pyi_hashes.json b/pyi_hashes.json index dc601e746f3..a8cb2ddc3bd 100644 --- a/pyi_hashes.json +++ b/pyi_hashes.json @@ -120,5 +120,5 @@ "packages/reflex-components-sonner/src/reflex_components_sonner/toast.pyi": "58521fcd1b514804f534d97624e82c9a", "reflex/__init__.pyi": "56385a4f0d9431eb0056dbc5553a58f9", "reflex/components/__init__.pyi": "9facd05a776d0641432696bbf8e34388", - "reflex/experimental/memo.pyi": "58d97de180cc34a8aa605cd76d97ee57" + "reflex/experimental/memo.pyi": "00c9ab0ff3086150278fb330953eeee8" } diff --git a/reflex/app.py b/reflex/app.py index 9c0d2af8db0..86a33155bc1 100644 --- a/reflex/app.py +++ b/reflex/app.py @@ -44,7 +44,7 @@ from reflex_base.event.processor import BaseStateEventProcessor, EventProcessor from reflex_base.registry import RegistrationContext from reflex_base.telemetry_context import CompileTrigger, TelemetryContext -from reflex_base.utils import console +from reflex_base.utils import console, memo_paths from reflex_base.utils.imports import ImportVar from reflex_base.utils.types import ASGIApp, Message, Receive, Scope, Send from reflex_components_core.base.error_boundary import ErrorBoundary @@ -283,6 +283,7 @@ class UnevaluatedPage: on_load: EventType[()] | None = None meta: Sequence[Mapping[str, Any] | Component] = () context: Mapping[str, Any] = dataclasses.field(default_factory=dict) + _source_module: str | None = None def merged_with(self, other: UnevaluatedPage) -> UnevaluatedPage: """Merge the other page into this one. @@ -301,6 +302,9 @@ def merged_with(self, other: UnevaluatedPage) -> UnevaluatedPage: else other.description, on_load=self.on_load if self.on_load is not None else other.on_load, context=self.context if self.context is not None else other.context, + _source_module=self._source_module + if self._source_module is not None + else other._source_module, ) @@ -915,6 +919,13 @@ def add_page( # Check if the route given is valid verify_route_validity(route) + if isinstance(component, Callable): + source_module = memo_paths.capture_source_module(component) + else: + # The user passed a pre-built Component instance — fall back to + # walking the call stack from add_page's caller. + source_module = memo_paths.resolve_user_module_from_frame(skip=1) + unevaluated_page = UnevaluatedPage( component=component, route=route, @@ -924,6 +935,7 @@ def add_page( on_load=on_load, meta=meta, context=context or {}, + _source_module=source_module, ) if route in self._unevaluated_pages: diff --git a/reflex/compiler/compiler.py b/reflex/compiler/compiler.py index 31ce5166ab0..f0185d793d3 100644 --- a/reflex/compiler/compiler.py +++ b/reflex/compiler/compiler.py @@ -2,6 +2,8 @@ from __future__ import annotations +import collections +import dataclasses import json import sys from collections.abc import Callable, Iterable, Sequence @@ -22,6 +24,7 @@ MemoDefinition, MemoFunctionDefinition, create_component_memo, + reset_memo_component_classes, ) from reflex_base.config import get_config from reflex_base.constants.compiler import PageNames, ResetStylesheet @@ -29,9 +32,10 @@ from reflex_base.environment import environment from reflex_base.plugins import CompileContext, CompilerHooks, PageContext, Plugin from reflex_base.style import SYSTEM_COLOR_MODE +from reflex_base.utils import memo_paths from reflex_base.utils.exceptions import ReflexError from reflex_base.utils.format import to_title_case -from reflex_base.utils.imports import ImportVar +from reflex_base.utils.imports import ABSOLUTE_IMPORT_PREFIXES, ImportVar from reflex_base.vars.base import LiteralVar, Var from reflex_components_core.base.app_wrap import AppWrap from reflex_components_core.base.fragment import Fragment @@ -81,11 +85,7 @@ def _extend_imports_in_place( for lib, fields in ( import_dict if isinstance(import_dict, tuple) else import_dict.items() ): - lib = ( - "$" + lib - if lib.startswith(("/utils/", "/components/", "/styles/", "/public/")) - else lib - ) + lib = "$" + lib if lib.startswith(ABSOLUTE_IMPORT_PREFIXES) else lib target_fields = target.setdefault(lib, []) if isinstance(fields, (list, tuple, set)): target_fields.extend( @@ -399,49 +399,137 @@ def _compile_component(component: Component) -> str: return templates.component_template(component=component) +@dataclasses.dataclass +class _MemoGroup: + """Accumulator for memos that share a mirrored output path.""" + + components: list[dict] = dataclasses.field(default_factory=list) + functions: list[dict] = dataclasses.field(default_factory=list) + imports: dict[str, list[ImportVar]] = dataclasses.field(default_factory=dict) + dynamic_imports: list[str] = dataclasses.field(default_factory=list) + custom_code: list[str] = dataclasses.field(default_factory=list) + + def add_component( + self, render: dict, memo_imports: dict[str, list[ImportVar]] + ) -> None: + self.components.append(render) + _extend_imports_in_place(self.imports, memo_imports) + self.dynamic_imports.extend(sorted(render.get("dynamic_imports", []) or [])) + self.custom_code.extend(render.get("custom_code", []) or []) + + def add_function( + self, render: dict, memo_imports: dict[str, list[ImportVar]] + ) -> None: + self.functions.append(render) + _extend_imports_in_place(self.imports, memo_imports) + + +# Imports every memo module needs regardless of its body: ``memo`` from React +# for the wrapper, and ``isTrue`` for prop coercion. Shared by the grouped and +# un-mirrored compile paths so they can't drift apart. +_MEMO_BASE_IMPORTS: dict[str, list[ImportVar]] = { + "react": [ImportVar(tag="memo")], + f"$/{constants.Dirs.STATE_PATH}": [ImportVar(tag="isTrue")], +} + + def _compile_memo_components( memos: Iterable[MemoDefinition] = (), ) -> tuple[list[tuple[str, str]], dict[str, list[ImportVar]]]: - """Compile each memo as its own module. + """Compile memos grouped by their source module's mirrored output path. - Each memo lands in ``.web//.jsx`` with only the imports - it actually uses. Memo wrappers declare their ``library`` as that - per-memo file path so page-side imports resolve directly to the - individual module. + Memos that captured a source module land in a single combined file at + ``.web/app_components/.jsx`` so the page-side import surface + matches the source layout. Memos that can't be mirrored (``__main__``, + unsafe module names) fall back to one file per memo at + ``.web/utils/components/.jsx`` that page-side code imports directly. Args: memos: The memos to compile. Returns: - A list of ``(path, code)`` pairs to write — one per memo — and the - aggregated imports across all memo modules. + A list of ``(path, code)`` pairs to write and the aggregated imports + across all memo modules. """ - per_memo_files: list[tuple[str, str]] = [] + output_files: list[tuple[str, str]] = [] aggregate_imports: dict[str, list[ImportVar]] = {} + unmirrored_files: list[tuple[str, str]] = [] + unmirrored_base_dir = utils.get_memo_components_dir() + groups: collections.defaultdict[tuple[str, ...], _MemoGroup] = ( + collections.defaultdict(_MemoGroup) + ) - base_dir = utils.get_memo_components_dir() + def _emit_unmirrored( + compile_fn: Callable[[dict, dict], tuple[str, dict[str, list[ImportVar]]]], + render: dict, + render_imports: dict, + ) -> None: + code, file_imports = compile_fn(render, render_imports) + unmirrored_files.append(( + _memo_component_file_path(unmirrored_base_dir, render["name"]), + code, + )) + _extend_imports_in_place(aggregate_imports, file_imports) for memo in memos: if isinstance(memo, MemoComponentDefinition): memo_render, memo_imports = utils.compile_experimental_component_memo(memo) - name = memo_render["name"] - code, file_imports = _compile_single_memo_component( - memo_render, memo_imports - ) - path = _memo_component_file_path(base_dir, name) - per_memo_files.append((path, code)) - _extend_imports_in_place(aggregate_imports, file_imports) + segments = memo_paths.module_to_mirrored_segments(memo.source_module) + if segments is None: + _emit_unmirrored( + _compile_single_memo_component, memo_render, memo_imports + ) + else: + groups[segments].add_component(memo_render, memo_imports) elif isinstance(memo, MemoFunctionDefinition): memo_render, memo_imports = utils.compile_experimental_function_memo(memo) - name = memo_render["name"] - code, file_imports = _compile_single_memo_function( - memo_render, memo_imports + segments = memo_paths.module_to_mirrored_segments(memo.source_module) + if segments is None: + _emit_unmirrored( + _compile_single_memo_function, memo_render, memo_imports + ) + else: + groups[segments].add_function(memo_render, memo_imports) + + if groups: + _extend_imports_in_place(aggregate_imports, _MEMO_BASE_IMPORTS) + _apply_common_imports(aggregate_imports) + + # Maps a case-folded output path to the module that claimed it, so two + # modules differing only by case (which collide on case-insensitive + # filesystems) are caught instead of one silently overwriting the other. + claimed_paths: dict[str, str] = {} + for segments, group in groups.items(): + module_path = utils.get_memo_module_path(segments) + module_name = ".".join(segments) + case_key = module_path.casefold() + if (clash := claimed_paths.get(case_key)) is not None: + msg = ( + f"Memoized component modules {clash!r} and {module_name!r} both " + f"mirror to {module_path!r} (their paths differ only by case), " + f"which collides on case-insensitive filesystems. Rename one of " + f"the source modules." ) - path = _memo_component_file_path(base_dir, name) - per_memo_files.append((path, code)) - _extend_imports_in_place(aggregate_imports, file_imports) + raise ReflexError(msg) + claimed_paths[case_key] = module_name + # Strip self-imports — when memos in this group reference each other, + # their compiled imports point at this group's own mirrored specifier. + # Importing from the same file would shadow the module's own exports. + self_specifier = memo_paths.mirrored_library_specifier(segments) + group.imports.pop(self_specifier, None) + merged_imports = utils.merge_imports(_MEMO_BASE_IMPORTS, group.imports) + _apply_common_imports(merged_imports) + code = templates.memo_components_template( + imports=utils.compile_imports(merged_imports), + components=group.components, + functions=group.functions, + dynamic_imports=sorted(set(group.dynamic_imports)), + custom_codes=list(dict.fromkeys(group.custom_code)), + ) + output_files.append((module_path, code)) + _extend_imports_in_place(aggregate_imports, group.imports) - return per_memo_files, aggregate_imports + return [*unmirrored_files, *output_files], aggregate_imports def _compile_single_memo_component( @@ -458,13 +546,7 @@ def _compile_single_memo_component( Returns: The file contents and the full import dict used to compile it. """ - imports = utils.merge_imports( - { - "react": [ImportVar(tag="memo")], - f"$/{constants.Dirs.STATE_PATH}": [ImportVar(tag="isTrue")], - }, - component_imports, - ) + imports = utils.merge_imports(_MEMO_BASE_IMPORTS, component_imports) _apply_common_imports(imports) code = templates.memo_single_component_template( imports=utils.compile_imports(imports), @@ -509,18 +591,6 @@ def _memo_component_file_path(base_dir: str, name: str) -> str: return str(Path(base_dir) / f"{name}{constants.Ext.JSX}") -def _memo_component_index_specifier(name: str) -> str: - """Return the module specifier the index uses to re-export a memo. - - Args: - name: The memo's export name. - - Returns: - A relative specifier resolvable from the memo index module. - """ - return f"./{constants.PageNames.COMPONENTS}/{name}" - - def compile_document_root( head_components: list[Component], html_lang: str | None = None, @@ -1083,6 +1153,10 @@ def compile_app( config.plugins, ) reset_bundled_libraries() + # Drop cached memo wrapper classes so each compile recomputes a memo's + # ``library`` from the current module layout (handles a module flipping to + # a package across hot reloads). + reset_memo_component_classes() for plugin in compiler_plugins: for dependency in plugin.get_frontend_dependencies(): bundle_library(dependency) @@ -1170,9 +1244,9 @@ def compile_app( hydrate_fallback_definition = create_component_memo( hydrate_fallback, "hydrate_fallback" ) - compile_ctx.auto_memo_components[hydrate_fallback_definition.export_name] = ( - hydrate_fallback_definition - ) + compile_ctx.auto_memo_components[ + hydrate_fallback_definition.export_name, None + ] = hydrate_fallback_definition hydrate_fallback_export = hydrate_fallback_definition.export_name memo_component_files, memo_components_imports = compile_memo_components( @@ -1275,6 +1349,10 @@ def add_save_task( if dry_run: return True + # Delete memo files this compile no longer emits. Done here (not before the + # dry-run return) so ``--dry`` never mutates ``.web`` or the manifest. + utils.prune_stale_memo_files(path for path, _ in memo_component_files) + with console.timing("Install Frontend Packages"): app._get_frontend_packages(all_imports) diff --git a/reflex/compiler/plugins/builtin.py b/reflex/compiler/plugins/builtin.py index de31f8c83c7..4081b4b9d79 100644 --- a/reflex/compiler/plugins/builtin.py +++ b/reflex/compiler/plugins/builtin.py @@ -205,6 +205,7 @@ def eval_page( name=getattr(page_fn, "__name__", page.route), route=page.route, root_component=component, + source_module=getattr(page, "_source_module", None), ) diff --git a/reflex/compiler/plugins/memoize.py b/reflex/compiler/plugins/memoize.py index 3eb4bc51873..36aee008ae7 100644 --- a/reflex/compiler/plugins/memoize.py +++ b/reflex/compiler/plugins/memoize.py @@ -9,8 +9,9 @@ Each unique subtree shape contributes: -- One generated experimental memo component definition, compiled into its own - per-memo module at ``$/utils/components/``. +- One generated experimental memo component definition, compiled into a module + mirroring its source module under ``$/app_components/`` (or, when it can't be + mirrored, a per-memo module at ``$/utils/components/``). - ``useCallback`` hook lines for each non-lifecycle event trigger, emitted into the generated memo body so handler hooks stay inside that rendering domain. @@ -381,10 +382,15 @@ def _build_wrapper( # ``create_passthrough_component_memo`` after the ``{children}`` hole # is substituted, so passthrough wrappers that differ only in their # children collapse to a single definition. - wrapper_factory, definition = create_passthrough_component_memo(comp) + wrapper_factory, definition = create_passthrough_component_memo( + comp, source_module=page_context.source_module + ) tag = definition.export_name compile_context.memoize_wrappers[tag] = None - compile_context.auto_memo_components[tag] = definition + # Key by (tag, source_module) so identical-rendering subtrees from + # different user modules each get their own entry and emit into the + # right mirrored memo file. + compile_context.auto_memo_components[tag, definition.source_module] = definition wrapper = wrapper_factory() # The wrapper has no structural children at the page level, but parents diff --git a/reflex/compiler/utils.py b/reflex/compiler/utils.py index 6daf1390a44..fb537e4d176 100644 --- a/reflex/compiler/utils.py +++ b/reflex/compiler/utils.py @@ -5,9 +5,12 @@ import asyncio import concurrent.futures import copy +import json import operator +import os +import tempfile import traceback -from collections.abc import Mapping, Sequence +from collections.abc import Iterable, Mapping, Sequence from datetime import datetime from pathlib import Path from typing import Any, TypedDict @@ -22,7 +25,7 @@ ) from reflex_base.constants.state import FIELD_MARKER from reflex_base.style import Style -from reflex_base.utils import format, imports +from reflex_base.utils import format, imports, memo_paths from reflex_base.utils.imports import ImportVar, ParsedImportDict from reflex_base.vars.base import Field, Var, VarData from reflex_base.vars.function import DestructuredArg @@ -420,10 +423,10 @@ def compile_experimental_component_memo( dynamic_imports = render._get_all_dynamic_imports() all_imports = render._get_all_imports() - # Each memo lives in ``web/utils/components/.jsx`` and is imported - # from ``$/utils/components/``. Strip a self-import so a memo body - # that references the wrapper's own module specifier doesn't recurse. - self_module = f"$/{constants.Dirs.COMPONENTS_PATH}/{definition.export_name}" + # Each un-mirrored memo lives in ``web/utils/components/.jsx`` and is + # imported from ``$/utils/components/``. Strip a self-import so a memo + # body that references its own specifier doesn't recurse. + self_module = memo_paths.unmirrored_library_specifier(definition.export_name) imports: ParsedImportDict = { lib: fields for lib, fields in all_imports.items() if lib != self_module } @@ -446,7 +449,9 @@ def compile_experimental_component_memo( return ( { "kind": "component", - "name": definition.export_name, + "name": memo_paths.library_and_symbol( + definition.source_module, definition.export_name + )[1], "signature": DestructuredArg( fields=tuple(signature_fields), rest=rest_param.placeholder_name if rest_param is not None else None, @@ -529,9 +534,9 @@ def compile_experimental_function_memo( # Reading ``.function`` evaluates a deferred function-memo body on first use. function = definition.function if var_data := function._get_all_var_data(): - # Per-file memo modules live at ``$/utils/components/``; strip - # only a self-import to this function memo's own module. - self_module = f"$/{constants.Dirs.COMPONENTS_PATH}/{definition.python_name}" + # Un-mirrored per-file memo modules live at ``$/utils/components/``; + # strip only a self-import to this function memo's own module. + self_module = memo_paths.unmirrored_library_specifier(definition.python_name) imports = { lib: list(fields) for lib, fields in dict(var_data.imports).items() @@ -541,7 +546,9 @@ def compile_experimental_function_memo( return ( { "kind": "function", - "name": definition.python_name, + "name": memo_paths.library_and_symbol( + definition.source_module, definition.python_name + )[1], "function": str(function), }, imports, @@ -744,15 +751,26 @@ def get_context_path() -> str: def get_memo_components_dir() -> str: - """Get the directory that holds per-memo module files. + """Get the directory that holds un-mirrored per-memo module files. Returns: - The directory used for per-memo ``.jsx`` modules. Pages import each - wrapper directly from ``$/utils/components/``. + The directory used for memos that can't be mirrored to a source module. + Pages import each wrapper directly from ``$/utils/components/``. """ - return str( - get_web_dir() / constants.Dirs.UTILS / constants.PageNames.COMPONENTS, - ) + return str(get_web_dir() / constants.Dirs.COMPONENTS_PATH) + + +def get_memo_module_path(segments: tuple[str, ...]) -> str: + """Get the on-disk path for a memo module mirrored from a Python module. + + Args: + segments: Mirrored path segments produced by + :func:`reflex_base.utils.memo_paths.module_to_mirrored_segments`. + + Returns: + The absolute path the compiler should write the combined memo file to. + """ + return str(memo_paths.mirrored_jsx_path(get_web_dir(), segments)) def add_meta( @@ -819,6 +837,96 @@ def write_file(path: str | Path, code: str): path.write_text(code, encoding="utf-8") +_MEMO_MANIFEST_FILENAME = ".memo-manifest.json" + + +def _read_memo_manifest(web_dir: Path) -> set[str]: + """Read the previous compile's memo file manifest. + + Args: + web_dir: The project's ``.web`` directory. + + Returns: + The set of paths (relative to ``.web``) recorded by the previous + compile, or an empty set if the manifest is absent or invalid. + """ + manifest_path = web_dir / _MEMO_MANIFEST_FILENAME + if not manifest_path.exists(): + return set() + try: + data = json.loads(manifest_path.read_text(encoding="utf-8")) + except (OSError, ValueError): + return set() + if not isinstance(data, list): + return set() + return {entry for entry in data if isinstance(entry, str)} + + +def _write_memo_manifest(web_dir: Path, relative_paths: set[str]) -> None: + """Atomically write the new memo file manifest. + + Args: + web_dir: The project's ``.web`` directory. + relative_paths: Paths emitted this run, relative to ``.web``. + """ + manifest_path = web_dir / _MEMO_MANIFEST_FILENAME + web_dir.mkdir(parents=True, exist_ok=True) + fd, tmp_name = tempfile.mkstemp( + prefix=".memo-manifest.", suffix=".json.tmp", dir=str(web_dir) + ) + # Close the raw fd immediately and reopen the file by path. Wrapping the + # fd via os.fdopen() would leak it if the wrap itself raised. + os.close(fd) + tmp_path = Path(tmp_name) + try: + with tmp_path.open("w", encoding="utf-8") as fh: + json.dump(sorted(relative_paths), fh) + tmp_path.replace(manifest_path) + except Exception: + # Best-effort cleanup; manifest write is recoverable on the next run. + tmp_path.unlink(missing_ok=True) + raise + + +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: Paths the current compile produced for the memo + pipeline, as built by joining :func:`get_web_dir` (so they share + its prefix — relative by default, absolute when overridden). + """ + web_dir = get_web_dir() + + # Emitted paths are built by joining ``web_dir`` (see Args), so each is + # always under it — no need to guard ``relative_to``. + emitted_relative = { + str(Path(path).relative_to(web_dir)).replace(os.sep, "/") + for path in emitted_paths + } + + previous = _read_memo_manifest(web_dir) + for relative in previous - emitted_relative: + target = web_dir / relative + if target.is_file(): + target.unlink() + parent = target.parent + while parent != web_dir and parent.is_relative_to(web_dir): + try: + parent.rmdir() + except OSError: + break + parent = parent.parent + + if emitted_relative != previous: + _write_memo_manifest(web_dir, emitted_relative) + + def empty_dir(path: str | Path, keep_files: list[str] | None = None): """Remove all files and folders in a directory except for the keep_files. diff --git a/tests/integration/test_auto_memo.py b/tests/integration/test_auto_memo.py index 121184f493e..25601755f17 100644 --- a/tests/integration/test_auto_memo.py +++ b/tests/integration/test_auto_memo.py @@ -55,7 +55,7 @@ def test_auto_memo_shared_across_pages(auto_memo_app: AppHarness): web_sources = "\n".join( path.read_text() for path in (auto_memo_app.app_path / ".web").rglob("*.jsx") ) - assert "$/utils/components" in web_sources + assert "$/app_components" in web_sources assert "$/utils/stateful_components" not in web_sources driver = auto_memo_app.frontend() diff --git a/tests/units/compiler/test_memoize_plugin.py b/tests/units/compiler/test_memoize_plugin.py index 19c25c42fb7..b9fb1c5df51 100644 --- a/tests/units/compiler/test_memoize_plugin.py +++ b/tests/units/compiler/test_memoize_plugin.py @@ -21,6 +21,7 @@ ) from reflex_base.constants.compiler import MemoizationDisposition, MemoizationMode from reflex_base.plugins import CompileContext, CompilerHooks, PageContext +from reflex_base.utils import memo_paths from reflex_base.vars import VarData from reflex_base.vars.base import Field, LiteralVar, Var, field from reflex_components_core.base.bare import Bare @@ -133,6 +134,7 @@ class FakePage: description: Any = None image: str = "" meta: tuple[dict[str, Any], ...] = () + _source_module: str | None = None def _compile_single_page( @@ -217,7 +219,7 @@ def test_memoize_wrapper_uses_memo_component_and_call_site() -> None: assert len(ctx.memoize_wrappers) == 1 wrapper_tag = next(iter(ctx.memoize_wrappers)) - assert wrapper_tag in ctx.auto_memo_components + assert (wrapper_tag, None) in ctx.auto_memo_components output = page_ctx.output_code or "" assert f'import {{{wrapper_tag}}} from "$/utils/components/{wrapper_tag}"' in output assert f"jsx({wrapper_tag}," in (page_ctx.output_code or "") @@ -245,7 +247,7 @@ def test_memoize_wrapper_deduped_across_repeated_subtrees() -> None: ) assert len(ctx.memoize_wrappers) == 1 wrapper_tag = next(iter(ctx.memoize_wrappers)) - assert list(ctx.auto_memo_components) == [wrapper_tag] + assert list(ctx.auto_memo_components) == [(wrapper_tag, None)] assert (page_ctx.output_code or "").count( f'import {{{wrapper_tag}}} from "$/utils/components/{wrapper_tag}"' ) == 1 @@ -615,8 +617,14 @@ def test_passthrough_memo_definitions_are_not_shared_globally(monkeypatch) -> No lambda comp, page_context: comp, ) - def fake_create_passthrough_component_memo(component: Component): - definition = SimpleNamespace(export_name=tag, component=component) + def fake_create_passthrough_component_memo( + component: Component, source_module: str | None = None + ): + definition = SimpleNamespace( + export_name=tag, + component=component, + source_module=source_module, + ) return (lambda definition=definition: definition), definition monkeypatch.setattr( @@ -627,7 +635,7 @@ def fake_create_passthrough_component_memo(component: Component): first_compile = SimpleNamespace(memoize_wrappers={}, auto_memo_components={}) second_compile = SimpleNamespace(memoize_wrappers={}, auto_memo_components={}) - page_context = cast(PageContext, SimpleNamespace()) + page_context = cast(PageContext, SimpleNamespace(source_module=None)) MemoizeStatefulPlugin._build_wrapper( first_component, @@ -640,8 +648,8 @@ def fake_create_passthrough_component_memo(component: Component): compile_context=second_compile, ) - first_definition = first_compile.auto_memo_components[tag] - second_definition = second_compile.auto_memo_components[tag] + first_definition = first_compile.auto_memo_components[tag, None] + second_definition = second_compile.auto_memo_components[tag, None] assert first_definition.component is first_component assert second_definition.component is second_component assert second_definition is not first_definition @@ -661,13 +669,84 @@ def test_shared_subtree_across_pages_uses_same_tag() -> None: assert len(ctx.memoize_wrappers) == 1 tag = next(iter(ctx.memoize_wrappers)) - assert list(ctx.auto_memo_components) == [tag] + assert list(ctx.auto_memo_components) == [(tag, None)] for route in ("/a", "/b"): output = ctx.compiled_pages[route].output_code or "" assert f'import {{{tag}}} from "$/utils/components/{tag}"' in output assert f"jsx({tag}," in output +def test_shared_subtree_in_distinct_source_modules_emits_per_module() -> None: + """Identical subtrees in different user modules emit one memo per module. + + Regression: the auto-memo registry was keyed by tag only, so when two + pages from different user modules produced the same memoizable subtree + (and therefore the same tag), the second registration overwrote the + first. Only the surviving definition's source module got a mirrored + memo file, leaving the other page importing a tag from a file that + never exported it. + """ + from reflex.compiler.compiler import compile_memo_components + + ctx = CompileContext( + pages=[ + FakePage( + route="/a", + component=lambda: Plain.create(STATE_VAR), + _source_module="memo_collision_test.module_a", + ), + FakePage( + route="/b", + component=lambda: Plain.create(STATE_VAR), + _source_module="memo_collision_test.module_b", + ), + ], + hooks=CompilerHooks(plugins=default_page_plugins()), + ) + with ctx: + ctx.compile() + + assert len(ctx.memoize_wrappers) == 1 + tag = next(iter(ctx.memoize_wrappers)) + # Both source modules survive registration as distinct entries. + assert set(ctx.auto_memo_components) == { + (tag, "memo_collision_test.module_a"), + (tag, "memo_collision_test.module_b"), + } + + # Each module emits and imports the tag under a per-module symbol so the + # two pages never collide on a shared JS identifier. + symbol_a = memo_paths.mirrored_symbol(tag, "memo_collision_test.module_a") + symbol_b = memo_paths.mirrored_symbol(tag, "memo_collision_test.module_b") + page_a_output = ctx.compiled_pages["/a"].output_code or "" + page_b_output = ctx.compiled_pages["/b"].output_code or "" + assert ( + f'import {{{symbol_a}}} from "$/app_components/memo_collision_test/module_a"' + in page_a_output + ) + assert ( + f'import {{{symbol_b}}} from "$/app_components/memo_collision_test/module_b"' + in page_b_output + ) + + output_files, _ = compile_memo_components( + memos=tuple(ctx.auto_memo_components.values()), + ) + emitted = {Path(path).as_posix(): code for path, code in output_files} + + def find_emitted(suffix: str) -> str | None: + return next( + (code for path, code in emitted.items() if path.endswith(suffix)), None + ) + + matched_a = find_emitted("memo_collision_test/module_a.jsx") + matched_b = find_emitted("memo_collision_test/module_b.jsx") + assert matched_a is not None, f"missing module_a memo file in {sorted(emitted)}" + assert matched_b is not None, f"missing module_b memo file in {sorted(emitted)}" + assert f"export const {symbol_a} = memo" in matched_a + assert f"export const {symbol_b} = memo" in matched_b + + def test_shared_parent_instance_across_pages_preserves_original() -> None: """A parent instance reused across pages must not have its children rebound. @@ -2206,11 +2285,11 @@ def test_restricted_content_element_with_id_and_stateful_child_still_memoizes() def test_each_memo_wrapper_emits_one_component_module_file() -> None: - """Every wrapper tag corresponds to exactly one ``components/{tag}.jsx`` file. + """Every wrapper tag corresponds to exactly one ``utils/components/{tag}.jsx`` file. - Locks the per-wrapper file invariant: ``compile_memo_components`` must - emit one module per wrapper (plus the shared index), so that React can - code-split per wrapper. A wrapper without a file (or a file without a + Locks the per-wrapper file invariant for un-mirrored memos: + ``compile_memo_components`` must emit one module per wrapper, so that React + can code-split per wrapper. A wrapper without a file (or a file without a wrapper) would mean broken imports at runtime. """ from reflex.compiler.compiler import compile_memo_components diff --git a/tests/units/compiler/test_plugins.py b/tests/units/compiler/test_plugins.py index 08943270a3b..351d9cdfccb 100644 --- a/tests/units/compiler/test_plugins.py +++ b/tests/units/compiler/test_plugins.py @@ -1172,7 +1172,7 @@ def test_compile_context_memoize_wrappers_registers_shared_subtree_tag() -> None # Both pages share the same subtree hash, so exactly one wrapper tag is registered. assert len(compile_ctx.memoize_wrappers) == 1 wrapper_tag = next(iter(compile_ctx.memoize_wrappers)) - assert list(compile_ctx.auto_memo_components) == [wrapper_tag] + assert [key for key, _ in compile_ctx.auto_memo_components] == [wrapper_tag] # Each page imports the generated experimental memo component. page_a_code = compile_ctx.compiled_pages["/a"].output_code or "" assert ( @@ -1194,7 +1194,7 @@ def test_compile_context_resets_memoize_wrappers_between_runs() -> None: with ctx: ctx.compile() first_tags = set(ctx.memoize_wrappers) - first_defs = set(ctx.auto_memo_components) + first_defs = {tag for tag, _ in ctx.auto_memo_components} assert first_tags # memoize wrapper was registered assert first_defs == first_tags @@ -1208,7 +1208,7 @@ def test_compile_context_resets_memoize_wrappers_between_runs() -> None: # Same shared component → same tag, not a union across runs. assert set(ctx2.memoize_wrappers) == first_tags - assert set(ctx2.auto_memo_components) == first_tags + assert {tag for tag, _ in ctx2.auto_memo_components} == first_tags page_ctx = ctx2.compiled_pages["/c"] assert "react-moment" in page_ctx.frontend_imports assert "$/utils/stateful_components" not in (page_ctx.output_code or "") diff --git a/tests/units/compiler/test_stale_cleanup.py b/tests/units/compiler/test_stale_cleanup.py new file mode 100644 index 00000000000..500fc02861c --- /dev/null +++ b/tests/units/compiler/test_stale_cleanup.py @@ -0,0 +1,169 @@ +"""Tests for the memo manifest-driven stale file cleanup.""" + +from __future__ import annotations + +import json +from pathlib import Path +from unittest.mock import patch + +import pytest + +from reflex.compiler import utils as compiler_utils + + +@pytest.fixture +def fake_web_dir(tmp_path: Path): + """Pretend tmp_path is the project's .web directory. + + Args: + tmp_path: The pytest tmp directory. + + Yields: + The path used as ``.web`` for the duration of the test. + """ + web_dir = tmp_path / ".web" + web_dir.mkdir() + with patch.object(compiler_utils, "get_web_dir", return_value=web_dir): + yield web_dir + + +@pytest.fixture +def relative_web_dir(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): + """Use a relative ``.web`` dir, matching the default get_web_dir() value. + + The other tests mock an absolute web dir, which hides path-joining bugs + that only surface when ``.web`` is the default relative path. + + Args: + tmp_path: The pytest tmp directory. + monkeypatch: The pytest monkeypatch fixture. + + Yields: + The relative ``.web`` path used for the duration of the test. + """ + monkeypatch.chdir(tmp_path) + web_dir = Path(".web") + web_dir.mkdir() + with patch.object(compiler_utils, "get_web_dir", return_value=web_dir): + yield web_dir + + +def _seed_manifest(web_dir: Path, paths: list[str]) -> None: + (web_dir / compiler_utils._MEMO_MANIFEST_FILENAME).write_text( + json.dumps(paths), encoding="utf-8" + ) + + +def _seed_files(web_dir: Path, relative_paths: list[str]) -> list[Path]: + written: list[Path] = [] + for rel in relative_paths: + target = web_dir / rel + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text("// memo", encoding="utf-8") + written.append(target) + return written + + +def test_prune_removes_files_dropped_between_runs(fake_web_dir: Path): + survivor, removed = _seed_files( + fake_web_dir, + ["myapp/widgets/buttons.jsx", "myapp/dropped.jsx"], + ) + _seed_manifest(fake_web_dir, ["myapp/widgets/buttons.jsx", "myapp/dropped.jsx"]) + + compiler_utils.prune_stale_memo_files([survivor]) + + assert survivor.exists() + assert not removed.exists() + + +def test_prune_cleans_empty_parent_dirs(fake_web_dir: Path): + survivor, _orphan = _seed_files( + fake_web_dir, + ["myapp/keep.jsx", "myapp/widgets/orphan.jsx"], + ) + _seed_manifest(fake_web_dir, ["myapp/keep.jsx", "myapp/widgets/orphan.jsx"]) + + compiler_utils.prune_stale_memo_files([survivor]) + + assert not (fake_web_dir / "myapp" / "widgets").exists() + assert (fake_web_dir / "myapp").exists() + + +def test_prune_only_touches_manifest_paths(fake_web_dir: Path): + untouched = fake_web_dir / "user_added.jsx" + untouched.write_text("// stays", encoding="utf-8") + [survivor] = _seed_files(fake_web_dir, ["myapp/keep.jsx"]) + # Manifest only mentions the survivor — even if other files exist next to + # it, prune must never delete files outside the manifest's history. + _seed_manifest(fake_web_dir, ["myapp/keep.jsx"]) + + compiler_utils.prune_stale_memo_files([survivor]) + + assert untouched.exists() + assert survivor.exists() + + +def test_prune_writes_new_manifest(fake_web_dir: Path): + [survivor] = _seed_files(fake_web_dir, ["myapp/widgets/buttons.jsx"]) + _seed_manifest(fake_web_dir, []) + + compiler_utils.prune_stale_memo_files([survivor]) + + manifest = json.loads( + (fake_web_dir / compiler_utils._MEMO_MANIFEST_FILENAME).read_text( + encoding="utf-8" + ) + ) + assert manifest == ["myapp/widgets/buttons.jsx"] + + +def test_prune_handles_missing_previous_manifest(fake_web_dir: Path): + [survivor] = _seed_files(fake_web_dir, ["myapp/widgets/buttons.jsx"]) + + # No manifest seeded — should not raise and should still write one. + compiler_utils.prune_stale_memo_files([survivor]) + + assert (fake_web_dir / compiler_utils._MEMO_MANIFEST_FILENAME).exists() + + +def test_prune_handles_relative_web_dir(relative_web_dir: Path): + survivor, renamed_from = _seed_files( + relative_web_dir, + ["myapp/keep.jsx", "myapp/old_name.jsx"], + ) + _seed_manifest(relative_web_dir, ["myapp/keep.jsx", "myapp/old_name.jsx"]) + # The rename target is written fresh this run and was never in the manifest. + [renamed_to] = _seed_files(relative_web_dir, ["myapp/new_name.jsx"]) + + # The compiler emits ``.web``-prefixed paths (str(get_web_dir() / ...)). With + # a relative ``.web`` the old is-absolute guard double-prefixed these to + # ``.web/.web/...``, so emitted keys never matched the manifest: the survivor + # and rename target were wrongly pruned and the new manifest was corrupted. + emitted = [ + str(relative_web_dir / "myapp" / "keep.jsx"), + str(relative_web_dir / "myapp" / "new_name.jsx"), + ] + compiler_utils.prune_stale_memo_files(emitted) + + assert survivor.exists() + assert renamed_to.exists() + assert not renamed_from.exists() + + manifest = json.loads( + (relative_web_dir / compiler_utils._MEMO_MANIFEST_FILENAME).read_text( + encoding="utf-8" + ) + ) + assert manifest == ["myapp/keep.jsx", "myapp/new_name.jsx"] + + +def test_prune_ignores_corrupt_manifest(fake_web_dir: Path): + (fake_web_dir / compiler_utils._MEMO_MANIFEST_FILENAME).write_text( + "not json", encoding="utf-8" + ) + [survivor] = _seed_files(fake_web_dir, ["myapp/widgets/buttons.jsx"]) + + compiler_utils.prune_stale_memo_files([survivor]) + + assert survivor.exists() diff --git a/tests/units/components/markdown/test_markdown.py b/tests/units/components/markdown/test_markdown.py index d51d3666f4d..855a8cafe20 100644 --- a/tests/units/components/markdown/test_markdown.py +++ b/tests/units/components/markdown/test_markdown.py @@ -2,6 +2,7 @@ from reflex_base.components.component import Component from reflex_base.components.memo import memo from reflex_base.plugins import CompileContext, CompilerHooks, PageContext +from reflex_base.utils import memo_paths from reflex_base.vars.base import Var from reflex_components_code.code import CodeBlock from reflex_components_code.shiki_code_block import ShikiHighLevelCodeBlock @@ -15,6 +16,10 @@ from reflex.compiler import compiler from reflex.compiler.plugins import default_page_plugins +# The ``code_block`` memo built by ``syntax_highlighter_memoized_component`` is +# defined in this module, so it compiles to a per-module-unique symbol. +_CODE_BLOCK_MEMO_SYM = memo_paths.mirrored_symbol("CodeBlock", __name__) + class CustomMarkdownComponent(Component, MarkdownComponentMap): """A custom markdown component.""" @@ -176,12 +181,16 @@ def test_create_map_fn_var_subclass(cls, fn_body, fn_args, explicit_return, expe ( "pre", {"pre": syntax_highlighter_memoized_component(CodeBlock)}, - r"""(({node, ...rest}) => { const {node: childNode, className, children: components, ...props} = rest.children.props; const children = String(Array.isArray(components) ? components.join('\n') : components).replace(/\n$/, ''); const match = (className || '').match(/language-(?.*)/); let _language = match ? match[1] : ''; ; return jsx(CodeBlock,{code:children,language:_language,...props},); })""", + r"""(({node, ...rest}) => { const {node: childNode, className, children: components, ...props} = rest.children.props; const children = String(Array.isArray(components) ? components.join('\n') : components).replace(/\n$/, ''); const match = (className || '').match(/language-(?.*)/); let _language = match ? match[1] : ''; ; return jsx(""" + + _CODE_BLOCK_MEMO_SYM + + r""",{code:children,language:_language,...props},); })""", ), ( "pre", {"pre": syntax_highlighter_memoized_component(ShikiHighLevelCodeBlock)}, - r"""(({node, ...rest}) => { const {node: childNode, className, children: components, ...props} = rest.children.props; const children = String(Array.isArray(components) ? components.join('\n') : components).replace(/\n$/, ''); const match = (className || '').match(/language-(?.*)/); let _language = match ? match[1] : ''; ; return jsx(CodeBlock,{code:children,language:_language,...props},); })""", + r"""(({node, ...rest}) => { const {node: childNode, className, children: components, ...props} = rest.children.props; const children = String(Array.isArray(components) ? components.join('\n') : components).replace(/\n$/, ''); const match = (className || '').match(/language-(?.*)/); let _language = match ? match[1] : ''; ; return jsx(""" + + _CODE_BLOCK_MEMO_SYM + + r""",{code:children,language:_language,...props},); })""", ), ], ) diff --git a/tests/units/components/memo_fixtures/__init__.py b/tests/units/components/memo_fixtures/__init__.py new file mode 100644 index 00000000000..fcdaf65a851 --- /dev/null +++ b/tests/units/components/memo_fixtures/__init__.py @@ -0,0 +1,6 @@ +"""Real importable modules for cross-module ``@rx.memo`` collision tests. + +Each submodule defines memos with names that intentionally clash across modules +so tests can exercise the full define -> register -> compile -> validate_imports +pipeline with genuine distinct ``fn.__module__`` values (not monkeypatched). +""" diff --git a/tests/units/components/memo_fixtures/module_a.py b/tests/units/components/memo_fixtures/module_a.py new file mode 100644 index 00000000000..3d0039905ee --- /dev/null +++ b/tests/units/components/memo_fixtures/module_a.py @@ -0,0 +1,33 @@ +"""Cross-module memo fixture A. + +Defines memos whose names also exist in ``module_b`` and ``module_c`` so the +same export name compiles in more than one module. +""" + +import reflex as rx + + +@rx.memo +def my_widget(title: rx.Var[str]) -> rx.Component: + """A component memo named the same as memos in the sibling fixtures. + + Args: + title: The title to render. + + Returns: + A text component. + """ + return rx.text("module_a:", title) + + +@rx.memo +def my_value(x: rx.Var[int]) -> rx.Var[str]: + """A function memo named the same as one in ``module_b``. + + Args: + x: The value to format. + + Returns: + A prefixed string var. + """ + return "a:" + x.to(str) diff --git a/tests/units/components/memo_fixtures/module_b.py b/tests/units/components/memo_fixtures/module_b.py new file mode 100644 index 00000000000..d0fd9d75ddf --- /dev/null +++ b/tests/units/components/memo_fixtures/module_b.py @@ -0,0 +1,33 @@ +"""Cross-module memo fixture B. + +Mirrors ``module_a``'s memo names with different bodies so the two modules +produce distinct memos that share an export name. +""" + +import reflex as rx + + +@rx.memo +def my_widget(title: rx.Var[str]) -> rx.Component: + """Same name as ``module_a.my_widget`` but a different body. + + Args: + title: The title to render. + + Returns: + A heading component. + """ + return rx.heading("module_b:", title) + + +@rx.memo +def my_value(x: rx.Var[int]) -> rx.Var[str]: + """Same name as ``module_a.my_value`` but a different body. + + Args: + x: The value to format. + + Returns: + A prefixed string var. + """ + return "b:" + x.to(str) diff --git a/tests/units/components/memo_fixtures/module_c.py b/tests/units/components/memo_fixtures/module_c.py new file mode 100644 index 00000000000..3bfdd59e1aa --- /dev/null +++ b/tests/units/components/memo_fixtures/module_c.py @@ -0,0 +1,36 @@ +"""Cross-module memo fixture C. + +``consumer`` references ``module_a.my_widget`` (a cross-module memo-to-memo +dependency), and this module also defines its own ``my_widget`` sharing the name +with ``module_a``/``module_b``. Together these cover a grouped memo file that +both imports another module's memo and exports its own same-named one. +""" + +import reflex as rx +from tests.units.components.memo_fixtures import module_a + + +@rx.memo +def consumer(title: rx.Var[str]) -> rx.Component: + """A memo whose body depends on ``module_a.my_widget``. + + Args: + title: The title forwarded to the dependency. + + Returns: + A box wrapping ``module_a.my_widget``. + """ + return rx.box(module_a.my_widget(title=title), rx.text("c-consumer")) + + +@rx.memo +def my_widget(title: rx.Var[str]) -> rx.Component: + """Same name as ``module_a.my_widget`` but defined in this module. + + Args: + title: The title to render. + + Returns: + A text component. + """ + return rx.text("module_c:", title) diff --git a/tests/units/components/test_memo.py b/tests/units/components/test_memo.py index 9276bf248c2..9fa38e51801 100644 --- a/tests/units/components/test_memo.py +++ b/tests/units/components/test_memo.py @@ -25,8 +25,9 @@ ) from reflex_base.event import EventChain, EventHandler, no_args_event_spec from reflex_base.style import Style -from reflex_base.utils import console +from reflex_base.utils import console, memo_paths from reflex_base.utils import format as format_utils +from reflex_base.utils.exceptions import ReflexError from reflex_base.utils.imports import ImportVar from reflex_base.vars import VarData from reflex_base.vars.base import Var @@ -52,17 +53,18 @@ def format_price(amount: rx.Var[int], currency: rx.Var[str]) -> rx.Var[str]: price = Var(_js_expr="price", _var_type=int) currency = Var(_js_expr="currency", _var_type=str) + sym = memo_paths.mirrored_symbol("format_price", __name__) assert ( str(format_price(amount=price, currency=currency)) - == "(format_price(price, currency))" + == f"({sym}(price, currency))" ) assert ( str(format_price.call(amount=price, currency=currency)) - == "(format_price(price, currency))" + == f"({sym}(price, currency))" ) assert isinstance(format_price._as_var(), FunctionVar) - definition = MEMOS["format_price"] + definition = MEMOS["format_price", __name__] assert isinstance(definition, MemoFunctionDefinition) assert ( str(definition.function) == '((amount, currency) => ((currency+": $")+amount))' @@ -97,26 +99,28 @@ def my_card( ) component_again = my_card(title="World") + sym = memo_paths.mirrored_symbol("MyCard", __name__) assert isinstance(component, MemoComponent) assert len(component.children) == 2 assert component.get_props() == ("title", "foo") assert type(component) is type(component_again) - assert type(component).tag == "MyCard" - assert type(component).get_fields()["tag"].default == "MyCard" + assert type(component).tag == sym + assert type(component).get_fields()["tag"].default == sym rendered = component.render() - assert rendered["name"] == "MyCard" + assert rendered["name"] == sym assert 'title:"Hello"' in rendered["props"] assert 'foo:"extra"' in rendered["props"] assert 'className:"extra"' in rendered["props"] - definition = MEMOS["MyCard"] + definition = MEMOS["MyCard", __name__] assert isinstance(definition, MemoComponentDefinition) assert any(str(prop) == "rest" for prop in definition.component.special_props) files, _ = compiler.compile_memo_components(tuple(MEMOS.values())) code = "\n".join(c for _, c in files) - assert "export const MyCard = memo(({children, title:title" in code + assert f"export const {sym} = memo(" in code + assert "({children, title:title" in code assert "...rest" in code assert "jsx(RadixThemesBox,{...rest}" in code @@ -132,15 +136,17 @@ def conditional_slot( ) -> rx.Var[rx.Component]: return rx.cond(show, first, second) - definition = MEMOS["ConditionalSlot"] + definition = MEMOS["ConditionalSlot", __name__] assert isinstance(definition, MemoComponentDefinition) assert definition.component.render() == { "contents": "(showRxMemo ? firstRxMemo : secondRxMemo)" } + sym = memo_paths.mirrored_symbol("ConditionalSlot", __name__) files, _ = compiler.compile_memo_components(tuple(MEMOS.values())) code = "\n".join(c for _, c in files) - assert "export const ConditionalSlot = memo(({show:showRxMemo" in code + assert f"export const {sym} = memo(" in code + assert "({show:showRxMemo" in code assert "(showRxMemo ? firstRxMemo : secondRxMemo)" in code @@ -162,10 +168,11 @@ def merge_styles( assert '["color"] : "red"' in str(merged) assert '["className"] : "primary"' in str(merged) + sym = memo_paths.mirrored_symbol("merge_styles", __name__) files, _ = compiler.compile_memo_components(tuple(MEMOS.values())) code = "\n".join(c for _, c in files) assert ( - "export const merge_styles = (({base, ...overrides}) => ({...base, ...overrides}));" + f"export const {sym} = (({{base, ...overrides}}) => ({{...base, ...overrides}}));" in code ) @@ -198,7 +205,7 @@ def test_component_memo_rest_prop_merge_is_forwarded_as_rest_prop(): def primary_button(rest: rx.RestProp, *, label: rx.Var[str]) -> rx.Component: return rx.button(label, rest.merge({"className": "btn"})) - definition = MEMOS["PrimaryButton"] + definition = MEMOS["PrimaryButton", __name__] assert isinstance(definition, MemoComponentDefinition) # The merged value is accepted as a RestProp: lifted onto special_props @@ -252,9 +259,10 @@ def label_slot( assert '["children"]' in str(rendered) assert '["className"] : "slot"' in str(rendered) + sym = memo_paths.mirrored_symbol("label_slot", __name__) files, _ = compiler.compile_memo_components(tuple(MEMOS.values())) code = "\n".join(c for _, c in files) - assert "export const label_slot = (({children, label, ...rest}) => label);" in code + assert f"export const {sym} = (({{children, label, ...rest}}) => label);" in code def test_memo_munges_legacy_bare_type_param(): @@ -270,7 +278,7 @@ def bad_annotation(value: int) -> rx.Var[str]: assert "bad_annotation" in kwargs["feature_name"] assert "`value`" in kwargs["reason"] - definition = MEMOS["bad_annotation"] + definition = MEMOS["bad_annotation", __name__] assert isinstance(definition, MemoFunctionDefinition) (value_param,) = definition.params assert value_param.kind is MemoParamKind.VALUE @@ -291,7 +299,7 @@ def legacy_card(title: str, count: int = 3) -> rx.Component: assert "`title`" in reason assert "`count`" in reason - definition = MEMOS["LegacyCard"] + definition = MEMOS["LegacyCard", __name__] assert isinstance(definition, MemoComponentDefinition) assert {p.name: p.kind for p in definition.params} == { "title": MemoParamKind.VALUE, @@ -414,7 +422,10 @@ def dropper(title: rx.Var[str]) -> rx.Component: component.class_name = Var.create("leaks") # set past the call-site gate files, _ = compiler.compile_memo_components(tuple(MEMOS.values())) - code = next(c for path, c in files if path.endswith("Dropper.jsx")) + segments = memo_paths.module_to_mirrored_segments(__name__) + assert segments is not None + exp_path = compiler_utils.get_memo_module_path(segments) + code = next(c for path, c in files if path == exp_path) # No rest capture, and the root Box gets an empty props object. assert "...rest" not in code assert "className" not in code @@ -434,7 +445,10 @@ def rest_card(rest: rx.RestProp, *, title: rx.Var[str]) -> rx.Component: assert isinstance(component, MemoComponent) files, _ = compiler.compile_memo_components(tuple(MEMOS.values())) - code = next(c for path, c in files if path.endswith("RestCard.jsx")) + segments = memo_paths.module_to_mirrored_segments(__name__) + assert segments is not None + exp_path = compiler_utils.get_memo_module_path(segments) + code = next(c for path, c in files if path == exp_path) # Undeclared props are captured in ``...rest`` and spread onto the root, so # ``className``/``id`` actually reach the rendered element. assert "...rest" in code @@ -565,7 +579,7 @@ def lazy_box(value: rx.Var[str]) -> rx.Component: return rx.box(value) # Decoration registers the memo without running the body. - assert "LazyBox" in MEMOS + assert ("LazyBox", __name__) in MEMOS assert evaluated == [] # First instantiation triggers a single evaluation... @@ -587,7 +601,7 @@ def lazy_join(value: rx.Var[str]) -> rx.Var[str]: evaluated.append(1) return value - assert "lazy_join" in MEMOS + assert ("lazy_join", __name__) in MEMOS assert evaluated == [] # Calling a function memo references the imported var, not the body. @@ -595,7 +609,7 @@ def lazy_join(value: rx.Var[str]) -> rx.Var[str]: assert evaluated == [] # The compiler (reading ``.function``) triggers a single evaluation. - definition = MEMOS["lazy_join"] + definition = MEMOS["lazy_join", __name__] assert isinstance(definition, MemoFunctionDefinition) _ = definition.function assert evaluated == [1] @@ -697,7 +711,7 @@ def soft_children(children) -> rx.Component: mock_deprecate.assert_called_once() - definition = MEMOS["SoftChildren"] + definition = MEMOS["SoftChildren", __name__] assert isinstance(definition, MemoComponentDefinition) (children_param,) = definition.params assert children_param.name == "children" @@ -753,7 +767,7 @@ def test_memo_rejects_component_and_function_name_collision(): def foo_bar() -> rx.Component: return rx.box() - assert "FooBar" in MEMOS + assert ("FooBar", __name__) in MEMOS with pytest.raises(ValueError, match=r"name collision.*FooBar"): @@ -776,6 +790,36 @@ def foo__bar() -> rx.Component: return rx.box() +def test_same_module_same_name_shadow_is_last_wins(): + """Two memos sharing a name in one module: the later definition wins. + + This is plain Python shadowing — a second ``def`` of the same name rebinds + the module global — and the registry follows suit rather than erroring, + because a genuine shadow is indistinguishable from a hot-reload + re-registration (same type/python_name/module/qualname). The factory builds + two distinct function objects with identical identity metadata, exactly as + two module-level ``def shadow`` would. + """ + + def _make_shadow(marker: str): + def shadow() -> rx.Component: + return rx.text(marker) + + return shadow + + rx.memo(_make_shadow("first-shadow-body")) + rx.memo(_make_shadow("second-shadow-body")) + + shadow_keys = [k for k in MEMOS if isinstance(k, tuple) and k[0] == "Shadow"] + assert len(shadow_keys) == 1 + + definition = MEMOS["Shadow", __name__] + files, _ = compiler.compile_memo_components((definition,)) + code = "\n".join(c for _, c in files) + assert "second-shadow-body" in code + assert "first-shadow-body" not in code + + def test_memo_rejects_varargs(): """Experimental memos should reject *args and **kwargs.""" with pytest.raises(TypeError, match=r"\*args"): @@ -846,7 +890,7 @@ def bad_hook(value: rx.Var[str]) -> rx.Var[str]: ) # Decoration defers the body; reading ``.function`` surfaces the error. - definition = MEMOS["bad_hook"] + definition = MEMOS["bad_hook", __name__] assert isinstance(definition, MemoFunctionDefinition) with pytest.raises(TypeError, match="cannot depend on hooks"): _ = definition.function @@ -864,7 +908,7 @@ def bad_import(value: rx.Var[str]) -> rx.Var[str]: ) # Decoration defers the body; reading ``.function`` surfaces the error. - definition = MEMOS["bad_import"] + definition = MEMOS["bad_import", __name__] assert isinstance(definition, MemoFunctionDefinition) with pytest.raises(TypeError, match="not bundled"): _ = definition.function @@ -888,9 +932,109 @@ def my_card(children: rx.Var[rx.Component], *, title: rx.Var[str]) -> rx.Compone files, _ = compiler.compile_memo_components(tuple(MEMOS.values())) code = "\n".join(c for _, c in files) - assert "export const TextWrapper = memo(" in code - assert "export const format_price =" in code - assert "export const MyCard = memo(" in code + text_wrapper_sym = memo_paths.mirrored_symbol("TextWrapper", __name__) + format_price_sym = memo_paths.mirrored_symbol("format_price", __name__) + my_card_sym = memo_paths.mirrored_symbol("MyCard", __name__) + assert f"export const {text_wrapper_sym} = memo(" in code + assert f"export const {format_price_sym} =" in code + assert f"export const {my_card_sym} = memo(" in code + + +def test_compile_memo_components_groups_by_source_module(): + """Memos sharing a source module are concatenated into one mirrored file.""" + + @rx.memo + def grouped_first(title: rx.Var[str]) -> rx.Component: + return rx.text(title) + + @rx.memo + def grouped_second(title: rx.Var[str]) -> rx.Component: + return rx.heading(title) + + definition = MEMOS["GroupedFirst", __name__] + assert definition.source_module is not None + segments = memo_paths.module_to_mirrored_segments(definition.source_module) + assert segments is not None + + files, _ = compiler.compile_memo_components(tuple(MEMOS.values())) + exp_path = compiler_utils.get_memo_module_path(segments) + + grouped_files = [(path, code) for path, code in files if path == exp_path] + assert len(grouped_files) == 1 + code = grouped_files[0][1] + first_sym = memo_paths.mirrored_symbol("GroupedFirst", __name__) + second_sym = memo_paths.mirrored_symbol("GroupedSecond", __name__) + assert f"export const {first_sym} = memo(" in code + assert f"export const {second_sym} = memo(" in code + # The merged module must carry imports its memos use, not just the + # framework-level ones added by the compiler. + assert "RadixThemesText" in code + assert "RadixThemesHeading" in code + + +def test_compile_memo_components_falls_back_when_no_source_module(): + """Memos with no source module emit to the legacy per-name path.""" + legacy_definition = MemoComponentDefinition( + fn=lambda: None, + python_name="legacy_memo", + params=(), + export_name="LegacyMemo", + _component=_LazyBody.ready(rx.fragment()), + passthrough_hole_child=None, + ) + + files, _ = compiler.compile_memo_components((legacy_definition,)) + exp_path = compiler._memo_component_file_path( + compiler_utils.get_memo_components_dir(), "LegacyMemo" + ) + assert any(path == exp_path for path, _ in files) + + +def test_compile_memo_components_mirrors_underscore_module_without_error(): + """A module named ``_internal`` mirrors normally — no reserved-name restriction. + + There is no reserved memo output directory anymore, so a developer is free + to name a package ``_internal``; it simply mirrors to + ``app_components/_internal/...`` like any other module. + """ + definition = MemoComponentDefinition( + fn=lambda: None, + python_name="thing", + params=(), + export_name="Thing", + _component=_LazyBody.ready(rx.fragment()), + passthrough_hole_child=None, + source_module="_internal.widgets", + ) + + files, _ = compiler.compile_memo_components((definition,)) + exp_path = compiler_utils.get_memo_module_path(("_internal", "widgets")) + assert any(path == exp_path for path, _ in files) + + +def test_compile_memo_components_rejects_case_insensitive_path_collision(): + """Two modules whose mirrored paths differ only by case fail loudly. + + On case-insensitive filesystems (macOS/Windows) both would resolve to one + file, silently overwriting one memo module with the other. + """ + + def _definition(export_name: str, source_module: str) -> MemoComponentDefinition: + return MemoComponentDefinition( + fn=lambda: None, + python_name=export_name.lower(), + params=(), + export_name=export_name, + _component=_LazyBody.ready(rx.fragment()), + passthrough_hole_child=None, + source_module=source_module, + ) + + with pytest.raises(ReflexError, match="case"): + compiler.compile_memo_components(( + _definition("Upper", "casecollide.Widget"), + _definition("Lower", "casecollide.widget"), + )) def test_compile_memo_components_extends_imports_without_remerging( @@ -970,7 +1114,7 @@ def wrapper() -> rx.Component: assert "inner" not in experimental_component._get_all_imports() - definition = MEMOS["Wrapper"] + definition = MEMOS["Wrapper", __name__] assert isinstance(definition, MemoComponentDefinition) _, imports = compiler_utils.compile_experimental_component_memo(definition) assert "inner" in imports @@ -985,7 +1129,7 @@ def test_compile_experimental_component_memo_does_not_mutate_definition( def wrapper() -> rx.Component: return rx.box("hi") - definition = MEMOS["Wrapper"] + definition = MEMOS["Wrapper", __name__] assert isinstance(definition, MemoComponentDefinition) # Reading ``.component`` triggers the deferred body evaluation. assert definition.component.style == Style() @@ -1060,7 +1204,7 @@ def eh_memo( rx.input(on_change=event), ) - definition = MEMOS["EhMemo"] + definition = MEMOS["EhMemo", __name__] assert isinstance(definition, MemoComponentDefinition) event_param = next(p for p in definition.params if p.name == "event") assert event_param.kind is MemoParamKind.EVENT_TRIGGER @@ -1075,7 +1219,7 @@ def test_component_memo_accepts_bare_event_handler(): def bare_eh_memo(event: rx.EventHandler) -> rx.Component: return rx.button("click", on_click=event()) - definition = MEMOS["BareEhMemo"] + definition = MEMOS["BareEhMemo", __name__] assert isinstance(definition, MemoComponentDefinition) event_param = next(p for p in definition.params if p.name == "event") assert event_param.kind is MemoParamKind.EVENT_TRIGGER @@ -1426,20 +1570,80 @@ def recursive_box(items: rx.Var[list[int]]) -> rx.Component: rx.foreach(items, lambda item: recursive_box(items=items)), ) - assert "RecursiveBox" in MEMOS - definition = MEMOS["RecursiveBox"] + assert ("RecursiveBox", __name__) in MEMOS + definition = MEMOS["RecursiveBox", __name__] assert isinstance(definition, MemoComponentDefinition) files, _ = compiler.compile_memo_components(tuple(MEMOS.values())) - body_source = next( - code for path, code in files if path.endswith("RecursiveBox.jsx") - ) + # The memo mirrors to its source module's combined file (named after the + # module, not the memo), so look it up by that path rather than a per-name + # ``RecursiveBox.jsx``. + segments = memo_paths.module_to_mirrored_segments(definition.source_module) + assert segments is not None + exp_path = compiler_utils.get_memo_module_path(segments) + body_source = next(code for path, code in files if path == exp_path) # ``>= 2``: once for the export, once for the recursive foreach call site. assert body_source.count("RecursiveBox") >= 2 instance = recursive_box(items=Var(_js_expr="items", _var_type=list[int])) assert isinstance(instance, MemoComponent) - assert type(instance).tag == "RecursiveBox" + assert type(instance).tag == memo_paths.mirrored_symbol("RecursiveBox", __name__) + + +def test_self_referencing_memo_omits_self_import_from_aggregate(): + """A self-importing memo must not leak its own specifier into the aggregate. + + The mirrored module specifier a memo uses to reference itself must be + stripped from the aggregate import set returned to the frontend-package scan. + """ + + @rx.memo + def recursive_card(items: rx.Var[list[int]]) -> rx.Component: + return rx.box(rx.foreach(items, lambda item: recursive_card(items=items))) + + definition = MEMOS["RecursiveCard", __name__] + segments = memo_paths.module_to_mirrored_segments(definition.source_module) + assert segments is not None + self_specifier = memo_paths.mirrored_library_specifier(segments) + + _, aggregate_imports = compiler.compile_memo_components((definition,)) + assert self_specifier not in aggregate_imports + + +def test_reset_memo_component_classes_recomputes_stale_library(monkeypatch): + """Resetting the class cache re-resolves a memo's library. + + A module that flips to a package across hot-reload compiles keeps its name + but gains an ``index`` segment; without a reset the cached wrapper class + would keep serving the pre-flip specifier. + """ + from reflex_base.components.memo import ( + _get_memo_component_class, + reset_memo_component_classes, + ) + + reset_memo_component_classes() + monkeypatch.setattr( + memo_paths, "module_to_mirrored_segments", lambda module: ("pkgflip",) + ) + flat = _get_memo_component_class("Flip", source_module="pkgflip") + assert flat.library == "$/app_components/pkgflip" + + # The module is now a package: same name, segments gain ``index``. + monkeypatch.setattr( + memo_paths, "module_to_mirrored_segments", lambda module: ("pkgflip", "index") + ) + # Stale until the cache is cleared. + assert ( + _get_memo_component_class("Flip", source_module="pkgflip").library + == "$/app_components/pkgflip" + ) + reset_memo_component_classes() + assert ( + _get_memo_component_class("Flip", source_module="pkgflip").library + == "$/app_components/pkgflip/index" + ) + reset_memo_component_classes() def test_self_referencing_var_memo(): @@ -1450,7 +1654,7 @@ def recursive_count(n: rx.vars.NumberVar[int]) -> rx.Var[int]: recurse = cast("rx.vars.NumberVar[int]", recursive_count(n=n - 1)) return cast("rx.Var[int]", rx.cond(n.bool(), n + recurse, 0)) - definition = MEMOS["recursive_count"] + definition = MEMOS["recursive_count", __name__] assert isinstance(definition, MemoFunctionDefinition) assert "recursive_count" in str(definition.function) diff --git a/tests/units/components/test_memo_cross_module.py b/tests/units/components/test_memo_cross_module.py new file mode 100644 index 00000000000..9b7a6dc6673 --- /dev/null +++ b/tests/units/components/test_memo_cross_module.py @@ -0,0 +1,199 @@ +"""Cross-module ``@rx.memo`` collision tests driven through the real pipeline. + +These exercise the full define -> register -> compile_memo_components -> page +compile -> validate_imports chain across REAL fixture modules (see +``memo_fixtures/``), the integration point that isolated per-module unit tests +missed. Each test imports the fixtures fresh so import-time registration runs +again after ``preserve_memo_registries`` has snapshotted the global registry. +""" + +import dataclasses +import importlib +import sys +from collections.abc import Callable +from pathlib import Path +from typing import Any + +import pytest +from reflex_base.components.component import Component +from reflex_base.components.memo import MEMOS +from reflex_base.plugins import CompileContext, CompilerHooks, PageContext +from reflex_base.utils import memo_paths + +import reflex as rx +from reflex.compiler.plugins import default_page_plugins + +_FIXTURE_PKG = "tests.units.components.memo_fixtures" + + +@pytest.fixture(autouse=True) +def _restore_memo_registries(preserve_memo_registries): + """Snapshot and restore the global memo registry around each test.""" + + +def _fresh_import(*short_names: str) -> tuple[Any, ...]: + """Import fixture modules fresh so their memos re-register this test. + + ``preserve_memo_registries`` restores ``MEMOS`` between tests, but + ``sys.modules`` keeps the fixture modules cached, so a plain ``import`` would + be a silent no-op and the memos would be missing. Dropping the cached + fixture modules first forces decoration to run again. + + Args: + short_names: Fixture submodule names (e.g. ``"module_a"``). + + Returns: + The freshly imported modules, in the requested order. + """ + for key in [ + k for k in sys.modules if k == _FIXTURE_PKG or k.startswith(_FIXTURE_PKG + ".") + ]: + del sys.modules[key] + return tuple( + importlib.import_module(f"{_FIXTURE_PKG}.{name}") for name in short_names + ) + + +@dataclasses.dataclass(slots=True) +class _FakePage: + route: str + component: Callable[[], Component] + title: Any = None + description: Any = None + image: str = "" + meta: tuple[dict[str, Any], ...] = () + _source_module: str | None = None + + +def _compile_page(factory: Callable[[], Component]) -> PageContext: + """Compile a single page through the production single-pass pipeline. + + Args: + factory: Builds the page's component tree. + + Returns: + The compiled page context (``.output_code`` holds the page JS). + """ + ctx = CompileContext( + pages=[_FakePage(route="/p", component=factory)], + hooks=CompilerHooks(plugins=default_page_plugins()), + ) + with ctx: + ctx.compile() + return ctx.compiled_pages["/p"] + + +def test_same_name_component_memos_in_different_modules_both_register(): + """Same-named component memos in two modules coexist without a collision.""" + module_a, module_b = _fresh_import("module_a", "module_b") + + sources = { + d.source_module + for d in MEMOS.values() + if getattr(d, "export_name", None) == "MyWidget" + } + assert {module_a.__name__, module_b.__name__} <= sources + + +def test_page_using_two_same_name_component_memos_compiles(): + """A page using both same-named component memos imports each distinctly.""" + module_a, module_b = _fresh_import("module_a", "module_b") + + page = _compile_page( + lambda: rx.fragment( + module_a.my_widget(title="a"), module_b.my_widget(title="b") + ) + ) + output = page.output_code or "" + + lib_a, sym_a = memo_paths.library_and_symbol(module_a.__name__, "MyWidget") + lib_b, sym_b = memo_paths.library_and_symbol(module_b.__name__, "MyWidget") + assert sym_a != sym_b + assert f'import {{{sym_a}}} from "{lib_a}"' in output + assert f'import {{{sym_b}}} from "{lib_b}"' in output + + +def test_cross_module_function_memos_same_name_compile(): + """Same-named function memos in two modules import under distinct symbols.""" + module_a, module_b = _fresh_import("module_a", "module_b") + + page = _compile_page( + lambda: rx.fragment( + rx.text(module_a.my_value(x=1)), rx.text(module_b.my_value(x=2)) + ) + ) + output = page.output_code or "" + + lib_a, sym_a = memo_paths.library_and_symbol(module_a.__name__, "my_value") + lib_b, sym_b = memo_paths.library_and_symbol(module_b.__name__, "my_value") + assert sym_a != sym_b + assert f'import {{{sym_a}}} from "{lib_a}"' in output + assert f'import {{{sym_b}}} from "{lib_b}"' in output + + +def test_memo_depends_on_memo_across_modules_in_grouped_file(): + """A grouped memo file imports a dependency's symbol and exports its own.""" + from reflex.compiler.compiler import compile_memo_components + + module_a, module_c = _fresh_import("module_a", "module_c") + + files, _ = compile_memo_components(tuple(MEMOS.values())) + emitted = {Path(path).as_posix(): code for path, code in files} + code_c = next( + ( + code + for path, code in emitted.items() + if path.endswith("memo_fixtures/module_c.jsx") + ), + None, + ) + assert code_c is not None, f"missing module_c memo file in {sorted(emitted)}" + + lib_a, sym_a = memo_paths.library_and_symbol(module_a.__name__, "MyWidget") + _lib_c, sym_c = memo_paths.library_and_symbol(module_c.__name__, "MyWidget") + _lib_consumer, sym_consumer = memo_paths.library_and_symbol( + module_c.__name__, "Consumer" + ) + + # The dependency is imported from module_a's own file under module_a's + # symbol; the local widget and consumer are emitted as local consts (the + # group's own self-import is stripped), so the two never redeclare a symbol. + assert sym_a != sym_c + assert f'import {{{sym_a}}} from "{lib_a}"' in code_c + assert f"export const {sym_consumer} = memo(" in code_c + assert f"export const {sym_c} = memo(" in code_c + + +def test_three_modules_sharing_a_name_all_compile(): + """Three modules sharing a memo name all compile with distinct symbols.""" + module_a, module_b, module_c = _fresh_import("module_a", "module_b", "module_c") + + page = _compile_page( + lambda: rx.fragment( + module_a.my_widget(title="a"), + module_b.my_widget(title="b"), + module_c.my_widget(title="c"), + ) + ) + output = page.output_code or "" + + symbols = [] + for module in (module_a, module_b, module_c): + lib, sym = memo_paths.library_and_symbol(module.__name__, "MyWidget") + assert f'import {{{sym}}} from "{lib}"' in output + symbols.append(sym) + assert len(set(symbols)) == 3 + + +def test_hot_reload_reregistration_is_idempotent(): + """Reloading a module re-registers its memo in place, not as a duplicate.""" + (module_a,) = _fresh_import("module_a") + importlib.reload(module_a) + + widgets = [ + d + for d in MEMOS.values() + if getattr(d, "export_name", None) == "MyWidget" + and d.source_module == module_a.__name__ + ] + assert len(widgets) == 1 diff --git a/tests/units/test_app.py b/tests/units/test_app.py index c59bb62875a..efe2535754a 100644 --- a/tests/units/test_app.py +++ b/tests/units/test_app.py @@ -26,7 +26,7 @@ from reflex_base.plugins import CompileContext, CompilerHooks, PageContext from reflex_base.registry import RegistrationContext from reflex_base.style import Style -from reflex_base.utils import console, exceptions, format +from reflex_base.utils import console, exceptions, format, memo_paths from reflex_base.utils.imports import ImportVar from reflex_base.vars.base import computed_var from reflex_components_core.base.bare import Bare @@ -42,7 +42,7 @@ import reflex as rx from reflex import AdminDash, constants from reflex._upload import upload -from reflex.app import App, ComponentCallable +from reflex.app import App, ComponentCallable, default_overlay_component from reflex.compiler.compiler import ( _compile_app, _memoize_stateful_app_wraps, @@ -2090,6 +2090,25 @@ def _find_error_boundary_memo_tag(app_root_code: str) -> str: return match.group() +def _find_mirrored_memo_symbol(app_root_code: str, name: str) -> str: + """Extract a mirrored memo's per-module JS symbol from app-root JS. + + Framework ``@memo`` overlay wraps mirror to their defining module, so their + bare export name (e.g. ``DefaultOverlayComponents``) gains a short + per-module hash suffix in the compiled output. + + Args: + app_root_code: The generated app-root source. + name: The memo's bare export name. + + Returns: + The mirrored symbol the memo is referenced under in the output. + """ + match = re.search(rf"{re.escape(name)}_[0-9a-f]{{8}}", app_root_code) + assert match is not None, f"mirrored memo symbol for {name!r} not found in app root" + return match.group() + + def compile_page_context_for_app_wraps(component: Component): """Compile one component through the page plugin pipeline. @@ -2170,6 +2189,12 @@ def test_app_wrap_compile_theme( ) ].strip() error_boundary_tag = _find_error_boundary_memo_tag(function_app_definition) + toast_provider_tag = _find_mirrored_memo_symbol( + function_app_definition, "MemoizedToastProvider" + ) + overlay_tag = _find_mirrored_memo_symbol( + function_app_definition, "DefaultOverlayComponents" + ) expected = ( "function AppWrap({children}) {\n\n\n\n\n" "return (" @@ -2180,10 +2205,10 @@ def test_app_wrap_compile_theme( # ErrorBoundary memoized: on_error trigger -> own memo module. + "jsx(RadixThemesColorModeProvider,{}," + "jsx(Fragment,{}," - + "jsx(MemoizedToastProvider,{},)," + + f"jsx({toast_provider_tag},{{}},)," + "jsx(RadixThemesTheme,{accentColor:\"plum\",css:{...theme.styles.global[':root'], ...theme.styles.global.body}}," + "jsx(Fragment,{}," - + "jsx(DefaultOverlayComponents,{},)," + + f"jsx({overlay_tag},{{}},)," + "jsx(Fragment,{}," + "children" + "))))))))" @@ -2195,8 +2220,7 @@ def test_app_wrap_compile_theme( # the memoized ``onError`` handler instead of the AppWrap chain. memo_contents = ( web_dir - / constants.Dirs.UTILS - / constants.PageNames.COMPONENTS + / constants.Dirs.COMPONENTS_PATH / f"{error_boundary_tag}{constants.Ext.JSX}" ).read_text() assert "fallbackRender" in memo_contents @@ -2512,7 +2536,7 @@ def test_compile_writes_app_wrap_memo_components( compilable_app: tuple[App, Path], mocker, ) -> None: - """App-wrap memo components are emitted as per-memo modules.""" + """App-wrap memo components mirror to their defining module's output file.""" conf = rx.Config(app_name="testing") mocker.patch("reflex_base.config._get_config", return_value=conf) app, web_dir = compilable_app @@ -2520,12 +2544,55 @@ def test_compile_writes_app_wrap_memo_components( app.add_page(rx.box("Index"), route="/") app._compile() - # Per-memo modules live under .web/utils/components/; each memo wrapper - # declares its ``library`` as the per-memo file path, so pages import it - # directly. - memo_dir = web_dir / constants.Dirs.UTILS / constants.PageNames.COMPONENTS - assert (memo_dir / f"DefaultOverlayComponents{constants.Ext.JSX}").exists() - assert (memo_dir / f"MemoizedToastProvider{constants.Ext.JSX}").exists() + # The overlay/toast app wraps are framework ``@memo`` functions, so they + # mirror to their defining module under .web/app_components/ (rather than a + # shared file). Their export must land in some mirrored module there. + memo_sources = "\n".join( + path.read_text() + for path in (web_dir / constants.Dirs.APP_COMPONENTS).rglob( + f"*{constants.Ext.JSX}" + ) + ) + # Each overlay memo mirrors to its defining module, so its export carries a + # per-module symbol derived from that module. + overlay_symbol = memo_paths.mirrored_symbol( + "DefaultOverlayComponents", default_overlay_component.__module__ + ) + toast_symbol = memo_paths.mirrored_symbol( + "MemoizedToastProvider", _compile_app.__module__ + ) + assert f"export const {overlay_symbol} = memo" in memo_sources + assert f"export const {toast_symbol} = memo" in memo_sources + + +def test_compile_dry_run_does_not_prune_or_write_manifest( + compilable_app: tuple[App, Path], + mocker, +) -> None: + """A ``--dry`` compile must not delete memo files or rewrite the manifest.""" + from reflex.compiler import utils as compiler_utils + + conf = rx.Config(app_name="testing") + mocker.patch("reflex_base.config._get_config", return_value=conf) + app, web_dir = compilable_app + app.add_page(rx.box("Index"), route="/") + + # Seed a stale memo file plus a manifest entry that a real compile would + # prune (it is no longer emitted). + stale_rel = f"{constants.Dirs.COMPONENTS_PATH}/Stale{constants.Ext.JSX}" + stale = web_dir / stale_rel + stale.parent.mkdir(parents=True, exist_ok=True) + stale.write_text("// stale", encoding="utf-8") + manifest = web_dir / compiler_utils._MEMO_MANIFEST_FILENAME + manifest.write_text(json.dumps([stale_rel]), encoding="utf-8") + manifest_before = manifest.read_text(encoding="utf-8") + + app._compile(dry_run=True) + + assert stale.exists(), "dry run must not delete stale memo files" + assert manifest.read_text(encoding="utf-8") == manifest_before, ( + "dry run must not rewrite the memo manifest" + ) def test_compile_writes_upload_files_provider_app_wrap( @@ -2589,7 +2656,7 @@ class AppWrapState(State): assert "children" in app_wrap_fn[app_wrap_fn.index("jsx(StateProvider") :] # The extracted memo module carries the state hook instead. - memo_dir = web_dir / constants.Dirs.UTILS / constants.PageNames.COMPONENTS + memo_dir = web_dir / constants.Dirs.COMPONENTS_PATH assert any( "useContext(StateContexts" in memo_file.read_text() for memo_file in memo_dir.glob(f"*{constants.Ext.JSX}") @@ -2832,6 +2899,12 @@ def page(): ) ].strip() error_boundary_tag = _find_error_boundary_memo_tag(function_app_definition) + toast_provider_tag = _find_mirrored_memo_symbol( + function_app_definition, "MemoizedToastProvider" + ) + overlay_tag = _find_mirrored_memo_symbol( + function_app_definition, "DefaultOverlayComponents" + ) expected = ( "function AppWrap({children}) {\n\n\n\n\n" "return (" @@ -2844,10 +2917,10 @@ def page(): + 'jsx(RadixThemesText,{as:"p"},' + "jsx(RadixThemesColorModeProvider,{}," + "jsx(Fragment,{}," - + "jsx(MemoizedToastProvider,{},)," + + f"jsx({toast_provider_tag},{{}},)," + "jsx(Fragment2,{}," + "jsx(Fragment,{}," - + "jsx(DefaultOverlayComponents,{},)," + + f"jsx({overlay_tag},{{}},)," + "jsx(Fragment,{}," + "children" + "))))))))))" diff --git a/tests/units/test_testing.py b/tests/units/test_testing.py index 82a40a2dd26..53804ef60dc 100644 --- a/tests/units/test_testing.py +++ b/tests/units/test_testing.py @@ -97,7 +97,7 @@ def test_app_harness_initialize_clears_memo_registries( """ monkeypatch.setattr(reflex_cli, "_init", lambda **kwargs: None) - MEMOS["format_value"] = mock.sentinel.memo + MEMOS["format_value", None] = mock.sentinel.memo harness = AppHarness.create( root=tmp_path / "memo_app", @@ -107,7 +107,7 @@ def test_app_harness_initialize_clears_memo_registries( harness.app_module_path.parent.mkdir(parents=True, exist_ok=True) harness._initialize_app() - assert "format_value" not in MEMOS + assert ("format_value", None) not in MEMOS harness_mocks.get_and_validate_app.assert_called_once_with(reload=True) diff --git a/tests/units/utils/test_memo_paths.py b/tests/units/utils/test_memo_paths.py new file mode 100644 index 00000000000..dd7c0accb24 --- /dev/null +++ b/tests/units/utils/test_memo_paths.py @@ -0,0 +1,164 @@ +"""Tests for source-module capture and mirrored-path translation.""" + +from __future__ import annotations + +import importlib.util +from pathlib import Path +from unittest.mock import patch + +from reflex_base.utils import memo_paths + + +def _user_fn(): + """Stand-in for a user-defined function (defined in this test module).""" + + +def test_capture_source_module_returns_user_module(): + captured = memo_paths.capture_source_module(_user_fn) + # Module name depends on how pytest collected this module; either is fine + # so long as it isn't filtered. + assert captured is not None + assert "test_memo_paths" in captured + + +def test_capture_source_module_mirrors_framework_module(): + # Framework-defined callables now mirror to their real module name rather + # than being routed into the shared un-mirrored fallback file. + captured = memo_paths.capture_source_module(memo_paths.capture_source_module) + assert captured == "reflex_base.utils.memo_paths" + + +def test_capture_source_module_filters_main(): + fn = type("F", (), {"__module__": "__main__"}) + assert memo_paths.capture_source_module(fn) is None + + +def test_capture_source_module_filters_missing(): + assert memo_paths.capture_source_module(None) is None + fn = type("F", (), {"__module__": ""}) + assert memo_paths.capture_source_module(fn) is None + + +def test_module_to_mirrored_segments_module(): + spec = importlib.util.find_spec("reflex.app") + # Ensure the test runs against a real, non-package module. + assert spec is not None + assert spec.origin + assert not spec.origin.endswith("__init__.py") + segments = memo_paths.module_to_mirrored_segments("reflex.app") + assert segments == ("reflex", "app") + + +def test_module_to_mirrored_segments_package_appends_index(): + # `reflex.components` is a package — its __init__.py origin should + # cause an "index" segment to be appended. + segments = memo_paths.module_to_mirrored_segments("reflex.components") + assert segments == ("reflex", "components", "index") + + +def test_module_to_mirrored_segments_unsafe_segment_rejected(): + assert memo_paths.module_to_mirrored_segments("foo..bar") is None + assert memo_paths.module_to_mirrored_segments("foo./bar") is None + assert memo_paths.module_to_mirrored_segments("..secret") is None + + +def test_module_to_mirrored_segments_rejects_windows_reserved_names(): + # Reserved device names (any case) can't be created as files on Windows, so + # a module with such a segment falls back to the un-mirrored path. + assert memo_paths.module_to_mirrored_segments("myapp.con") is None + assert memo_paths.module_to_mirrored_segments("aux") is None + assert memo_paths.module_to_mirrored_segments("myapp.COM1") is None + assert memo_paths.module_to_mirrored_segments("myapp.lpt9.ui") is None + # A name merely containing a reserved name is fine. + assert memo_paths.module_to_mirrored_segments("myapp.controls") == ( + "myapp", + "controls", + ) + + +def test_segment_is_safe_rejects_trailing_dot_or_space(): + assert memo_paths._segment_is_safe("ok") + assert not memo_paths._segment_is_safe("trailing.") + assert not memo_paths._segment_is_safe("trailing ") + + +def test_is_framework_module_covers_only_exact_packages(): + # The predicate recognizes framework packages by exact name or dot boundary, + # but now only steers the frame-walk fallback (resolve_user_module_from_frame), + # not mirroring. + assert memo_paths._is_framework_module("reflex") + assert memo_paths._is_framework_module("reflex.app") + assert memo_paths._is_framework_module("reflex_base") + # The reflex_components_* family is the convention community component + # libraries follow, so it is NOT treated as framework — those modules mirror + # to their real package name like any other. + assert not memo_paths._is_framework_module("reflex_components_radix") + # A module that merely starts with "reflex" but isn't a framework package. + assert not memo_paths._is_framework_module("reflexion.pages") + + +def test_capture_source_module_no_longer_filters_framework(): + def _fn_in(module: str): + return type("F", (), {"__module__": module}) + + # Framework modules are captured and mirrored to their real package name. + assert memo_paths.capture_source_module(_fn_in("reflex.app")) == "reflex.app" + assert ( + memo_paths.capture_source_module(_fn_in("reflex_components_radix")) + == "reflex_components_radix" + ) + # User modules are captured unchanged. + assert ( + memo_paths.capture_source_module(_fn_in("reflexion.pages")) == "reflexion.pages" + ) + + +def test_library_for_mirrors_or_falls_back(): + assert ( + memo_paths.library_for("counter_app.ui", "Button") + == "$/app_components/counter_app/ui" + ) + # No source module → per-name fallback path. + assert memo_paths.library_for(None, "Button") == "$/utils/components/Button" + # Unsafe module → per-name fallback path. + assert memo_paths.library_for("bad..name", "Button") == "$/utils/components/Button" + + +def test_module_to_mirrored_segments_none(): + assert memo_paths.module_to_mirrored_segments(None) is None + assert memo_paths.module_to_mirrored_segments("") is None + + +def test_mirrored_jsx_path_joins_segments(): + web_dir = Path("/tmp/.web") + path = memo_paths.mirrored_jsx_path(web_dir, ("counter_app", "ui", "buttons")) + assert path == web_dir / "app_components" / "counter_app" / "ui" / "buttons.jsx" + + +def test_mirrored_library_specifier_joins_with_slash(): + spec = memo_paths.mirrored_library_specifier(("counter_app", "ui", "buttons")) + assert spec == "$/app_components/counter_app/ui/buttons" + + +def test_mirrored_paths_live_under_reserved_app_components_dir(): + """User module paths are nested under app_components/, so they can't clobber framework output.""" + assert memo_paths.mirrored_jsx_path(Path(".web"), ("app", "root")) == Path( + ".web/app_components/app/root.jsx" + ) + assert ( + memo_paths.mirrored_library_specifier(("app", "root")) + == "$/app_components/app/root" + ) + + +def test_resolve_user_module_from_frame_skips_framework(): + captured = memo_paths.resolve_user_module_from_frame() + # Must find a user frame — the test module itself qualifies. + assert captured is not None + assert not captured.startswith("reflex") + + +def test_resolve_user_module_from_frame_returns_none_inside_framework_only(): + # Simulate a stack of framework-only frames. + with patch.object(memo_paths, "_is_framework_module", return_value=True): + assert memo_paths.resolve_user_module_from_frame() is None diff --git a/tests/units/utils/test_telemetry_accounting.py b/tests/units/utils/test_telemetry_accounting.py index 7fcd996d055..760c713571a 100644 --- a/tests/units/utils/test_telemetry_accounting.py +++ b/tests/units/utils/test_telemetry_accounting.py @@ -215,13 +215,10 @@ class UserWalkState(rx.State): def test_memo_wrapper_class_records_wrapped_component_type(): """The dynamic memo subclass exposes the user-authored component class.""" - import importlib - + from reflex_base.components.memo import _get_memo_component_class from reflex_components_radix.themes.components.button import Button - memo_module = importlib.import_module("reflex.experimental.memo") - - wrapper_cls = memo_module._get_memo_component_class( + wrapper_cls = _get_memo_component_class( "Button_button_deadbeefcafebabe", Button, )