Skip to content

some typing nits - #6722

Draft
benedikt-bartscher wants to merge 3 commits into
reflex-dev:mainfrom
benedikt-bartscher:port-typing-fixes
Draft

benedikt-bartscher wants to merge 3 commits into
reflex-dev:mainfrom
benedikt-bartscher:port-typing-fixes

Conversation

@benedikt-bartscher

Copy link
Copy Markdown
Contributor

No description provided.

@greptile-apps

greptile-apps Bot commented Jul 8, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Low impact] The PR appears safe to merge; no actionable defects were found.

Summary

This PR tightens types and cleans up two tests without an established behavior regression.

  • Tailwind plugin strings become imports in generated configs.
  • State field reads now have clearer collection type hints.
  • Event-chain invocations are typed as functions.
  • Tests keep their temporary files and handler swaps contained.

No actionable new issues were found. Tests were not run.

Reviews (3) · Last reviewed commit: "Merge remote-tracking branch 'upstream/m..." · Reviewed by Greptile

@codspeed

codspeed Bot commented Jul 8, 2026 •

Copy link
Copy Markdown

Merging this PR will not alter performance

⚠️ 17 benchmarks spent significant time in system calls

System calls cannot be consistently instrumented, so they are not included in the measure, which understates the real cost. Please switch to the Walltime instrument to accurately measure system calls.

Measurement and system calls

✅ 148 untouched benchmarks


Comparing benedikt-bartscher:port-typing-fixes (c230ebe) with main (4be8b4d)

Open in CodSpeed

Comment on lines -111 to +110
if isinstance(plugin, Mapping) and "import" in plugin
if not isinstance(plugin, str) and "import" in plugin

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

i dont like this one because it's a runtime check. despite the static typing, if this plugin object is not a str, it could be anything else and indexing into it with a str would be invalid. hence, checking if it's explicitly a Mapping makes the in check and subsequent __getitem__ call safe.

what was the motivation for this change? i might be misreading it.

# Conflicts:
#	packages/reflex-base/src/reflex_base/plugins/shared_tailwind.py
#	packages/reflex-base/src/reflex_base/vars/base.py
#	tests/units/test_state.py

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants