Repository navigation
tentative migrate pyright to ty - #6465
benedikt-bartscher wants to merge 49 commits into
Conversation
Merging this PR will not alter performance1 warning
Comparing |
Resolve conflicts from upstream's memo refactor (reflex-dev#6517, promote rx._x.memo to rx.memo): memo implementation moved to reflex_base.components.memo and CustomComponent was removed. Re-apply the ty migration changes onto the new locations: - reflex/experimental/memo.py: take upstream's deprecated alias - reflex_base/components/memo.py: type fn params as FunctionType and add ty:ignore for dynamic __signature__ assignment - tests: drop CustomComponent tests removed upstream; carry ty:ignore comments onto the relocated memo/telemetry tests - pyproject.toml / uv.lock: keep both towncrier (upstream) and ty (HEAD) - pyi_hashes.json: regenerated ty check, ruff, and the unit suite pass.
Conflict resolutions of note: - pyproject.toml: `[tool.pyright]` is replaced by the branch's `[tool.ty.*]` tables; main's new `packages/reflex-ui-shared` exclusion is folded in, and `[tool.check-outdated-deps] backend_pinned` now pins `ty` instead of `pyright`, which is what main's new `scripts/check_outdated_deps.py` reads (the branch's inline `grep -vE` filter in the workflow is gone). - .pre-commit-config.yaml: the repo-wide hook runs `ty check`; main's `tests/type_checking` hook keeps its own id so both stay addressable. - reflex_base logging: main's move from `console` to stdlib `logging` wins in decorator.py, tasks.py and types.py, keeping the branch's `callable_name` for names a `Callable` does not statically have. - vars/base.py: `field()`'s overloads keep main's `FIELD_TYPE` / `Field[FIELD_TYPE]` return types rather than the branch's `Any`; main's `tests/type_checking/fields.py` asserts them. - istate/proxy.py: main's `_dataclass_proxy_namespace` plus the wrapt-stubs cast in `__new__`, which ty needs as well. - scripts/release.py: deleted, as on main. Type checking is clean after converting main's new `pyright: ignore` and `type: ignore` comments to the ty rules ty actually reports.
[Medium impact] The migration reaches type-check workflows and shared Reflex code. Fix the chart dependency floor and remove the duplicate compiler check before merging. FindingsSummaryReplaces Pyright with
Reviews (7) · Last reviewed commit: "Regenerate pyi hashes after the upstream..." · Reviewed by Greptile |
`scripts/check_min_deps.py` still shelled out to `pyright`, which this branch removed from the dev dependencies, so every `check-min-deps` job died with `FileNotFoundError: 'pyright'` before checking anything. It now runs `ty check --python <isolated venv> --output-format concise` and diffs the parsed error lines, with the rule severities copied out of the root `[tool.ty.rules]` into the `ty.toml` that `--config-file` supplies, so the isolated runs stay comparable to the repo-wide check. ty resolves more of the older releases than pyright did, so several packages now report deltas against their declared `reflex-base` floors. Those floors are genuinely behind the `field()` signature the components use; raising them is left to a separate change.
Conflict resolutions of note: - pyproject.toml: main's `[tool.pyright] extraPaths` stays dropped; its `packages/reflex-workflow` entry, which the workflow examples import from, becomes `[tool.ty.environment] extra-paths`. - scripts/check_min_deps.py: main's workspace wheelhouse and per-module invocation, run through ty; the per-module test now inspects the ty command. - vars/base.py and state.py: main's descriptor-backed fields and attribute access win; `_update_substate_vars` keeps the branch's `Mapping` parameter, and the mixin copy keeps the `FunctionType` guard on `fget`. - istate/proxy.py: `ImmutableMutableProxy._self_state` stays annotated as `StateProxy`, so main's cast in `_dirty_state` is dropped. - reflex/constants/custom_components.py: deleted, as on main. Type checking is clean after converting main's new `pyright: ignore` and `type: ignore` comments to the ty rules ty actually reports, and ignoring ty's stricter `**` unpacking checks in main's new tests.
Comments Outside DiffThese findings could not be posted inline.
|
Conflicts keep upstream's refactors (proxy class cache, Var.to, redis state writes, file locks, Python 3.11 floor) with the branch's ty changes reapplied on top: pyright ignores dropped, ComputedVar.__get__ takes upstream's `instance: Any` with the branch's `owner: type[BaseState]`, reflex.model keeps SQLModelConfig beside upstream's lazy AsyncSession import. With 3.10 gone, the explicit ty python-version setting is dropped. New ty diagnostics in upstream code are fixed: the wrapt 2.5 wrapper takes an args tuple, the frontend shutdown flag is annotated bool, _cache_per_class names fn through callable_name, and the playground types its chart size dict and checks for a str before the filter membership test. Upstream pyright ignores that ty also reports became ty ignores, and ignores made unused by the fixed-arity EventCallback overloads are gone.
rx.Var.create(1).to(int) is a ToOperation, so the LiteralVar check missed it and get_var_value raised instead of returning 1.
format.py now imports console at module level, which closes an import cycle through log and decorator, so decorator.py imports callable_name where it is used. The new frozenset/set type forms in figure_out_type and the deprecated format_data_editor_column test call get ty ignores like their neighbours.
| from reflex_base.constants import EventTriggers | ||
| from reflex_base.constants.colors import Color | ||
| from reflex_base.event import EventHandler, no_args_event_spec | ||
| from reflex_base.utils.format import callable_name |
There was a problem hiding this comment.
Charts fail with allowed dependency
charts.py now imports callable_name, but reflex-components-recharts still allows reflex-base >= 0.10.0. That release lacks this helper, so users with that supported version get an ImportError when loading chart components. Raise the dependency floor to the first version containing the helper. The repository guide also requires raising sibling dependency floors when using unreleased changes.
Context Used: AGENTS.md (source)
| if isinstance(component, Component): | ||
| return component | ||
| if (converted := _into_component_once(component)) is not None: | ||
| return converted |
There was a problem hiding this comment.
Compiler repeats a component check
into_component() now checks for Component immediately before _into_component_once() performs the same check. Callable page inputs therefore pass the same check twice. The repository guide says not to repeat checks or add runtime workarounds for type-only problems. Remove this duplicate branch before merging to satisfy that requirement.
| if isinstance(component, Component): | |
| return component | |
| if (converted := _into_component_once(component)) is not None: | |
| return converted | |
| if (converted := _into_component_once(component)) is not None: | |
| return converted |
Context Used: AGENTS.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Uh oh!
There was an error while loading. Please reload this page.