-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
fix(callbacks): use . for dots in dict ID callback IDs to fix JSON.parse SyntaxError #3923
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
Mukller
wants to merge
3
commits into
plotly:dev
Choose a base branch
from
Mukller:fix/dict-id-dot-escape
base: dev
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+179
−132
Open
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,128 +1,165 @@ | ||
| """Unit tests for callback decorator behavior - no browser required.""" | ||
| import inspect | ||
|
|
||
| import dash | ||
| from dash import Input, Output, State, callback | ||
|
|
||
|
|
||
| def test_callback_returns_callable(): | ||
| """Test that callback returns a callable decorator.""" | ||
| decorator = callback(Output("output", "children"), Input("input", "value")) | ||
| assert callable(decorator) | ||
|
|
||
|
|
||
| def test_callback_decorates_function(): | ||
| """Test that callback can decorate a function.""" | ||
|
|
||
| @callback(Output("output", "children"), Input("input", "value")) | ||
| def my_callback(value): | ||
| return f"Value: {value}" | ||
|
|
||
| assert callable(my_callback) | ||
| assert my_callback.__name__ == "my_callback" | ||
|
|
||
|
|
||
| def test_callback_signature_includes_typed_options(): | ||
| """Test that callback exposes the expected decorator keyword arguments.""" | ||
| sig = inspect.signature(callback) | ||
|
|
||
| expected = { | ||
| "background", | ||
| "interval", | ||
| "progress", | ||
| "progress_default", | ||
| "running", | ||
| "cancel", | ||
| "manager", | ||
| "cache_args_to_ignore", | ||
| "cache_ignore_triggered", | ||
| "on_error", | ||
| "api_endpoint", | ||
| "optional", | ||
| "hidden", | ||
| } | ||
| assert expected.issubset(set(sig.parameters)) | ||
|
|
||
|
|
||
| def test_callback_with_multiple_inputs(): | ||
| """Test callback with multiple inputs.""" | ||
|
|
||
| @callback( | ||
| Output("output", "children"), | ||
| Input("input1", "value"), | ||
| Input("input2", "value"), | ||
| ) | ||
| def multi_input_callback(val1, val2): | ||
| return f"{val1} + {val2}" | ||
|
|
||
| assert callable(multi_input_callback) | ||
|
|
||
|
|
||
| def test_callback_with_state(): | ||
| """Test callback with State.""" | ||
|
|
||
| @callback( | ||
| Output("output", "children"), | ||
| Input("input", "value"), | ||
| State("state", "value"), | ||
| ) | ||
| def callback_with_state(input_val, state_val): | ||
| return f"{input_val} - {state_val}" | ||
|
|
||
| assert callable(callback_with_state) | ||
|
|
||
|
|
||
| def test_callback_with_multiple_outputs(): | ||
| """Test callback with multiple outputs.""" | ||
|
|
||
| @callback( | ||
| Output("output1", "children"), | ||
| Output("output2", "children"), | ||
| Input("input", "value"), | ||
| ) | ||
| def multi_output_callback(value): | ||
| return value, f"Copy: {value}" | ||
|
|
||
| assert callable(multi_output_callback) | ||
|
|
||
|
|
||
| def test_callback_preserves_docstring(): | ||
| """Test that callback preserves the wrapped function's docstring.""" | ||
|
|
||
| @callback(Output("output", "children"), Input("input", "value")) | ||
| def documented_callback(value): | ||
| """This is a documented callback.""" | ||
| return value | ||
|
|
||
| assert documented_callback.__doc__ == "This is a documented callback." | ||
|
|
||
|
|
||
| def test_callback_with_prevent_initial_call(): | ||
| """Test callback with prevent_initial_call parameter.""" | ||
|
|
||
| @callback( | ||
| Output("output", "children"), | ||
| Input("input", "value"), | ||
| prevent_initial_call=True, | ||
| ) | ||
| def callback_no_initial(value): | ||
| return value | ||
|
|
||
| assert callable(callback_no_initial) | ||
|
|
||
|
|
||
| def test_callback_with_background_params(): | ||
| """Test that callback accepts background callback parameters.""" | ||
| decorator = callback( | ||
| Output("output", "children"), | ||
| Input("input", "value"), | ||
| background=False, | ||
| interval=1000, | ||
| ) | ||
| assert callable(decorator) | ||
|
|
||
|
|
||
| def test_callback_module_export(): | ||
| """Test that callback is properly exported from dash module.""" | ||
| assert hasattr(dash, "callback") | ||
| assert dash.callback is callback | ||
| """Unit tests for callback decorator behavior - no browser required.""" | ||
| import inspect | ||
| import json | ||
|
|
||
| import dash | ||
| from dash import Input, Output, State, callback | ||
| from dash._utils import create_callback_id | ||
|
|
||
|
|
||
| def test_callback_returns_callable(): | ||
| """Test that callback returns a callable decorator.""" | ||
| decorator = callback(Output("output", "children"), Input("input", "value")) | ||
| assert callable(decorator) | ||
|
|
||
|
|
||
| def test_callback_decorates_function(): | ||
| """Test that callback can decorate a function.""" | ||
|
|
||
| @callback(Output("output", "children"), Input("input", "value")) | ||
| def my_callback(value): | ||
| return f"Value: {value}" | ||
|
|
||
| assert callable(my_callback) | ||
| assert my_callback.__name__ == "my_callback" | ||
|
|
||
|
|
||
| def test_callback_signature_includes_typed_options(): | ||
| """Test that callback exposes the expected decorator keyword arguments.""" | ||
| sig = inspect.signature(callback) | ||
|
|
||
| expected = { | ||
| "background", | ||
| "interval", | ||
| "progress", | ||
| "progress_default", | ||
| "running", | ||
| "cancel", | ||
| "manager", | ||
| "cache_args_to_ignore", | ||
| "cache_ignore_triggered", | ||
| "on_error", | ||
| "api_endpoint", | ||
| "optional", | ||
| "hidden", | ||
| } | ||
| assert expected.issubset(set(sig.parameters)) | ||
|
|
||
|
|
||
| def test_callback_with_multiple_inputs(): | ||
| """Test callback with multiple inputs.""" | ||
|
|
||
| @callback( | ||
| Output("output", "children"), | ||
| Input("input1", "value"), | ||
| Input("input2", "value"), | ||
| ) | ||
| def multi_input_callback(val1, val2): | ||
| return f"{val1} + {val2}" | ||
|
|
||
| assert callable(multi_input_callback) | ||
|
|
||
|
|
||
| def test_callback_with_state(): | ||
| """Test callback with State.""" | ||
|
|
||
| @callback( | ||
| Output("output", "children"), | ||
| Input("input", "value"), | ||
| State("state", "value"), | ||
| ) | ||
| def callback_with_state(input_val, state_val): | ||
| return f"{input_val} - {state_val}" | ||
|
|
||
| assert callable(callback_with_state) | ||
|
|
||
|
|
||
| def test_callback_with_multiple_outputs(): | ||
| """Test callback with multiple outputs.""" | ||
|
|
||
| @callback( | ||
| Output("output1", "children"), | ||
| Output("output2", "children"), | ||
| Input("input", "value"), | ||
| ) | ||
| def multi_output_callback(value): | ||
| return value, f"Copy: {value}" | ||
|
|
||
| assert callable(multi_output_callback) | ||
|
|
||
|
|
||
| def test_callback_preserves_docstring(): | ||
| """Test that callback preserves the wrapped function's docstring.""" | ||
|
|
||
| @callback(Output("output", "children"), Input("input", "value")) | ||
| def documented_callback(value): | ||
| """This is a documented callback.""" | ||
| return value | ||
|
|
||
| assert documented_callback.__doc__ == "This is a documented callback." | ||
|
|
||
|
|
||
| def test_callback_with_prevent_initial_call(): | ||
| """Test callback with prevent_initial_call parameter.""" | ||
|
|
||
| @callback( | ||
| Output("output", "children"), | ||
| Input("input", "value"), | ||
| prevent_initial_call=True, | ||
| ) | ||
| def callback_no_initial(value): | ||
| return value | ||
|
|
||
| assert callable(callback_no_initial) | ||
|
|
||
|
|
||
| def test_callback_with_background_params(): | ||
| """Test that callback accepts background callback parameters.""" | ||
| decorator = callback( | ||
| Output("output", "children"), | ||
| Input("input", "value"), | ||
| background=False, | ||
| interval=1000, | ||
| ) | ||
| assert callable(decorator) | ||
|
|
||
|
|
||
| def test_callback_module_export(): | ||
| """Test that callback is properly exported from dash module.""" | ||
| assert hasattr(dash, "callback") | ||
| assert dash.callback is callback | ||
|
|
||
|
|
||
| def test_create_callback_id_escapes_dots_in_string_id(): | ||
| """A dot in a plain string component id is escaped with a backslash.""" | ||
| output = Output("my.component", "children") | ||
| callback_id = create_callback_id(output, []) | ||
|
|
||
| assert callback_id == "my\\.component.children" | ||
|
|
||
|
|
||
| def test_create_callback_id_escapes_dots_in_dict_id_as_json_unicode(): | ||
| """A dot in a dict id must use the JSON \\u002e escape, not \\., | ||
| otherwise the frontend's JSON.parse throws a SyntaxError when it | ||
| un-escapes the id portion of the callback id string (see #3480).""" | ||
| output = Output({"type": "my.type", "index": 1}, "children") | ||
| callback_id = create_callback_id(output, []) | ||
|
|
||
| id_part, prop_part = callback_id.rsplit(".", 1) | ||
| assert prop_part == "children" | ||
| # The escaped id must not contain a raw backslash-dot sequence... | ||
| assert "\\." not in id_part | ||
| # ...and must be valid JSON once the . escape is present verbatim. | ||
| assert "\\u002e" in id_part | ||
| parsed = json.loads(id_part) | ||
| assert parsed == {"type": "my.type", "index": 1} | ||
|
|
||
|
|
||
| def test_create_callback_id_dict_id_without_dots_unaffected(): | ||
| """Dict ids with no dots in their values still round-trip through JSON.""" | ||
| output = Output({"type": "widget", "index": 2}, "value") | ||
| callback_id = create_callback_id(output, []) | ||
|
|
||
| id_part, prop_part = callback_id.rsplit(".", 1) | ||
| assert prop_part == "value" | ||
| assert json.loads(id_part) == {"type": "widget", "index": 2} | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file got converted to CRLF, which is why it shows as a full rewrite (132 deletions) instead of just the three added tests. Can you convert it back to LF so the diff is only the new tests?