Skip to content

fix: make disable_plugins accept Plugin types instead of strings#6155

Open
masenf wants to merge 1 commit intomainfrom
claude/hopeful-merkle
Open

fix: make disable_plugins accept Plugin types instead of strings#6155
masenf wants to merge 1 commit intomainfrom
claude/hopeful-merkle

Conversation

@masenf
Copy link
Collaborator

@masenf masenf commented Mar 5, 2026

Change disable_plugins from list[str] to list[type[Plugin]] so both plugins and disable_plugins use consistent Plugin-based types (#6150).

Strings and instances are still accepted at runtime with a deprecation warning, and normalized to the Plugin class. Environment variable support via REFLEX_DISABLE_PLUGINS continues to work through a new interpret_plugin_class_env interpreter, which interpret_plugin_env now reuses for class resolution.

Fix #6150

Change `disable_plugins` from `list[str]` to `list[type[Plugin]]` so both
`plugins` and `disable_plugins` use consistent Plugin-based types (#6150).

Strings and instances are still accepted at runtime with a deprecation
warning, and normalized to the Plugin class. Environment variable support
via REFLEX_DISABLE_PLUGINS continues to work through a new
`interpret_plugin_class_env` interpreter, which `interpret_plugin_env`
now reuses for class resolution.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@codspeed-hq
Copy link

codspeed-hq bot commented Mar 5, 2026

Merging this PR will not alter performance

✅ 8 untouched benchmarks


Comparing claude/hopeful-merkle (e1373b6) with main (e7c3742)

Open in CodSpeed

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 5, 2026

Greptile Summary

This PR migrates disable_plugins from list[str] to list[type[Plugin]], making it consistent with the plugins field and enabling type-safe, IDE-friendly configuration. Backward compatibility is preserved through runtime normalization with deprecation warnings, and environment variable support (REFLEX_DISABLE_PLUGINS) is maintained via the new interpret_plugin_class_env helper.

Key changes:

  • BaseConfig.disable_plugins type changed from list[str] to list[type[Plugin]]
  • Config._normalize_disable_plugins() added to coerce legacy string/instance entries to Plugin classes with deprecation warnings
  • interpret_plugin_class_env extracted from interpret_plugin_env so both class-resolution and instance-creation paths share logic
  • interpret_env_var_value now dispatches type[Plugin] annotated fields to interpret_plugin_class_env
  • _add_builtin_plugins updated to use class-based in checks instead of string matching
  • Comprehensive tests added for all normalization paths and the new env-var interpreter

One minor issue found: an inline comment in the normalization logic incorrectly describes the conversion direction.

Confidence Score: 4/5

  • Safe to merge; changes are well-tested and backward-compatible with only a minor inline comment issue.
  • The PR logic is sound, all backward-compatibility paths are covered by tests, and the env-var plumbing correctly handles the new list[type[Plugin]] type. One minor style issue found: a misleading inline comment in reflex/config.py that describes the conversion direction incorrectly.
  • reflex/config.py — fix misleading comment on line 356.

Last reviewed commit: e1373b6

for key, env_value in env_kwargs.items():
setattr(self, key, env_value)

# Normalize disable_plugins: convert strings and Plugin subclasses to instances.
Copy link
Contributor

Choose a reason for hiding this comment

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

The comment says "convert strings and Plugin subclasses to instances", but the actual logic does the opposite: it converts Plugin instances (and strings) to Plugin classes. The word "subclasses" and "instances" are swapped.

Suggested change
# Normalize disable_plugins: convert strings and Plugin subclasses to instances.
# Normalize disable_plugins: convert strings and Plugin instances to Plugin subclasses.

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.

Inconsistent types between plugins and disable_plugins in rx.Config (str vs Plugin)

1 participant