✅ Snyk checks have passed. No issues have been found so far.

Status	Scanner	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues
✅	Licenses	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

✅ PR preview is ready!

Summary

This PR introduces a reusable frontend useQueryParamBinding hook and integrates it into the existing useBasicWidgetState hook, then adds the bind="query-params" parameter to st.color_picker as the first widget to support two-way URL query parameter synchronization. The changes span the full stack: protobuf schema, Python backend, React frontend hooks, and comprehensive tests at all levels (unit + E2E).

Key changes:

• New useQueryParamBinding hook — registers/unregisters widget bindings with WidgetStateManager, with proper cleanup on unmount.
• useBasicWidgetState integration — accepts an optional queryParamBinding config, with careful memoization to prevent unnecessary effect re-runs.
• st.color_picker bind support — adds bind: BindOption = None keyword-only parameter, proto field query_param_key, and hex validation in the serde.
• Invalid value handling — _seed_widget_from_url now detects when a deserializer silently falls back to default for invalid URL values and clears the param.
• Validation hardening — StreamlitInvalidBindValueError for bad bind values, empty key rejection in require_valid_user_key.

Code Quality

Frontend — Well-structured and idiomatic React:

• The useQueryParamBinding hook follows the Effect checklist: it syncs with an external system (WidgetStateManager), has exactly one concern, includes cleanup, and has a complete dependency array. Good.
• The memoization in useBasicWidgetState (lines 259–279 of useBasicWidgetState.ts) is thoughtfully done — defaultValueForBinding uses useMemo keyed on element, and queryParamBindingOptions uses JSON.stringify for value-based array comparison. The eslint-disable comments explain the rationale.
• The QueryParamBindingConfig interface in useBasicWidgetState.ts is well-documented and forward-looking (includes urlFormat and optionStrings for future widgets).

Potential issue — removed eslint-disable comment:

In useBasicWidgetState.ts line 296, the mutation element.setValue = false previously had an eslint-disable comment for react-hooks/immutability. The diff shows this comment was removed:

-    // eslint-disable-next-line react-hooks/immutability -- TODO: Update to match React best practices
     element.setValue = false // Clear "event".

This may introduce a new lint warning/error. Please verify this doesn't break the lint step, or restore the disable comment if the rule is still active.

Backend Python — Clean and follows conventions:

• _HEX_COLOR_RE is compiled at module level with a leading underscore — follows the module-private convention.
• ColorPickerSerde.deserialize is enhanced to validate hex colors and normalize the # prefix, which is essential for graceful handling of URL-seeded values.
• The StreamlitInvalidBindValueError follows the existing LocalizableStreamlitException pattern exactly.

Proto change — Backward compatible:

• optional string query_param_key = 10; in ColorPicker.proto — field 10 is the next sequential number, optional correctly indicates presence semantics. Additive-only change; no existing fields are modified.

Test Coverage

Test coverage is excellent across all layers:

E2E Tests (5 new tests):

• URL seeding from query params
• URL updates on user interaction (including reset to default)
• Custom default with URL override
• Invalid URL value handling (cleared from URL)
• 3-char hex shorthand support

All tests use expect (auto-wait) assertions, descriptive names, docstrings, and label-based locators via get_color_picker from app_utils. Negative assertions are present (e.g., verifying query param absence when value returns to default, verifying invalid params are removed).

Frontend Unit Tests:

• useQueryParamBinding.test.ts (7 tests): register, unregister, unmount cleanup, null key, options passthrough, re-registration on key change.
• useBasicWidgetState.test.ts (4 new tests): integration with binding config, no-binding, unmount, options passthrough.
• ColorPicker.test.tsx (4 new tests): register on mount, unregister on unmount, no-op without key, custom default.

Python Unit Tests:

• color_picker_test.py: bind sets proto field, bind without key raises, no-bind leaves proto empty, invalid bind raises, empty key raises, serde validation.
• session_state_test.py: invalid URL value detection and clearing.
• widgets_test.py: invalid bind value validation, bind excluded from element ID computation.

Minor suggestions for additional coverage:

1. A TestColorPickerSerde test for the serialize method (trivial but documents round-trip behavior).
2. A test verifying that ColorPickerSerde.deserialize("") returns the default (the empty string path).

Backwards Compatibility

Low risk overall, but two changes affect all widgets:

1. check_widget_policies now calls require_valid_user_key(key) for all widgets (policies.py line 178-179). This means key="" will now raise a StreamlitAPIException for any widget, not just st.color_picker. While key="" is almost certainly a user bug (it would cause undefined behavior), this is technically a breaking change for anyone passing empty string keys. This is a reasonable hardening that prevents subtle bugs.

2. _seed_widget_from_url has new invalid-value detection (session_state.py lines 1028-1039). This affects all widgets with bind="query-params". The logic is sound: if the deserializer silently falls back to default for a non-default URL value, the param is cleared. The three-part condition (deserialized_value == default_value and parsed_value != serialized_default and not is_empty_url_value(url_value)) correctly distinguishes "invalid value fell back to default" from "user explicitly set the default value in the URL". However, this could be a subtle behavior change for any future widgets where multiple valid URL representations map to the default value but don't round-trip through serializer(deserializer(None)).

The bind parameter itself is keyword-only (after *), defaults to None, and has no effect when not set — fully backward compatible.

Security & Risk

• No security concerns identified. The hex color validation regex is correct and prevents injection of arbitrary strings. Invalid URL values are sanitized rather than passed through.
• Regression risk is low. The ColorPickerSerde.deserialize change is more restrictive than before (validates instead of blindly converting), which is safer. The previous behavior of str(ui_value) would have returned the raw string for any input — now invalid inputs fall back to default.
• The _seed_widget_from_url change adds an extra metadata.deserializer(None) and metadata.serializer(default_value) call per seeding operation. This is negligible overhead.

Accessibility

No accessibility concerns. The color picker's UI is unchanged — only the data binding layer is modified. The widget continues to use the same BaseColorPicker component with proper labeling and keyboard interaction.

Recommendations

1. Verify the eslint-disable removal: In useBasicWidgetState.ts, the react-hooks/immutability disable comment was removed from the element.setValue = false mutation (around line 296). If this lint rule is still active, this will cause a lint failure. Either restore the disable comment or address the TODO to avoid the mutation.

2. Consider adding a JSDoc note about referential stability for useQueryParamBinding: The options parameter is an object in the useEffect dependency array. While the current caller (useBasicWidgetState) properly memoizes it, direct callers of useQueryParamBinding need to be aware that passing a new object reference each render will cause unnecessary effect re-runs. A brief note in the JSDoc (e.g., "The options object should be memoized or stable across renders") would help future consumers.

3. Add serde tests for edge cases: Consider adding ColorPickerSerde unit tests for:

   • deserialize("") returns the default value
   • serialize round-trips correctly with deserialize
   • deserialize with a value like "##000000" (double hash) falls back to default

4. Document the broader impact of check_widget_policies change: The empty-key validation in policies.py affects all widgets globally. While this is a good hardening, it might be worth calling this out in the PR description or changelog since it could affect users passing key="" to any widget.

Verdict

APPROVED: This is a well-structured, thoroughly tested PR that introduces clean infrastructure for query param binding and applies it to st.color_picker as the first widget. The code follows existing patterns, test coverage is comprehensive across all layers, and the changes are backward compatible. The minor issues identified (eslint-disable removal, referential stability documentation) are non-blocking and can be addressed in follow-up if needed.

This is an automated AI review using opus-4.6-thinking. Please verify the feedback and use your judgment.

Summary

Adds query-param binding for st.color_picker, wires frontend registration through a reusable hook, hardens URL seeding/clearing logic, and expands unit + e2e coverage for the new behavior.

Code Quality

Overall structure is clean and consistent with existing widget patterns. One style issue: new Python tests added without -> None return annotations, which conflicts with the Python test guide (e.g., lib/tests/streamlit/elements/color_picker_test.py:236-263).

Test Coverage

Coverage is solid across backend, frontend hooks, and e2e scenarios (seeding, URL updates, invalid values, defaults). One e2e test does not include a negative “must NOT happen” assertion as recommended in the e2e guide (e2e_playwright/st_color_picker_test.py:202-210).

Backwards Compatibility

bind is optional and defaults to None, so existing code paths remain unchanged. The new validation will raise on invalid bind values, which is expected behavior and should be low-risk.

Security & Risk

No security concerns identified. URL normalization/cleanup is localized to query-param binding and appears low risk.

Accessibility

No UI or interaction changes beyond URL syncing; accessibility behavior should be unchanged.

Recommendations

1. Add a negative assertion to the query-param seeding e2e test to align with e2e best practices (e2e_playwright/st_color_picker_test.py:202-210).
2. Add explicit -> None return types to new Python tests to match the test guide (lib/tests/streamlit/elements/color_picker_test.py:236-263).
3. Since a proto field was added, ensure protobuf artifacts are regenerated if this repo tracks them (proto/streamlit/proto/ColorPicker.proto:21-32).

Verdict

APPROVED: Feature is well-implemented and tested; only minor best-practice nits remain.

This is an automated AI review using gpt-5.2-codex-high. Please verify the feedback and use your judgment.