error when autogenerated IDs are used with persistence or snapshots #1894
Conversation
also give set_random_id a leading underscore so it doesn't need to become a reserved word (disallowed prop name)
dash/development/base_component.py
Outdated
| including component IDs, and auto-generated IDs can easily change. | ||
| Callbacks referencing the new IDs will not work old snapshots. | ||
|
|
||
| Please assign an explicit ID to this component. |
There was a problem hiding this comment.
Is there any way for us to print out "which" component it is? Or any other identifier (besides the ID) like the component type? If a user has a ton of components or callbacks, it can be difficult for them to find out which component the error message is referring too.
There was a problem hiding this comment.
See the error string prefix in __init__ as an example - might be able to reuse that. the id itself might not be helpful, but at least seeing the component name and type
There was a problem hiding this comment.
good call, the callback definition will point to multiple components, including the type will help the user narrow it down.
| def _set_random_id(self): | ||
|
|
||
| if hasattr(self, "id"): | ||
| return getattr(self, "id") |
There was a problem hiding this comment.
😅 I had this logic wrong at first - the whole point of this PR is to force folks to add this ID explicitly, so if it's already there we should skip the checks below! Now that case is tested
| @@ -23,7 +23,6 @@ | |||
| "UNDEFINED", | |||
| "REQUIRED", | |||
| "to_plotly_json", | |||
| "set_random_id", | |||
There was a problem hiding this comment.
We really want to limit new additions to reserved_words, as these become ineligible as prop names for all dash components anyone ever makes. So I prepended an underscore to the method name.
| def test_debc029_random_id_errors(): | ||
| app = Dash(__name__) | ||
|
|
||
| input1 = dcc.Input(value="Hello Input 1", persistence=True) |
There was a problem hiding this comment.
I am pretty sure its probably late, and not sure if its valid here. But we can force the user to always Instantiate Input with a defined id by making it a required keyword-arg. That way it will simply be a runtime error.
class Input():
def __init__(self, value=None, style=None, *, id):
...
# The following line will throw an error
# The error states that __init__ is missing a required keyword-arg 'id'
input1 = Input(value="something", style={'color': 'orange'})
There was a problem hiding this comment.
There was a problem hiding this comment.
Basically anything after the * becomes a required keyword arg. and its part of python 3.6.
There was a problem hiding this comment.
Dash components do have the ability to specify required args - and this gets handled in each component's __init__ via the component generator. The challenge here is that it's coupled, though you're right that in principle we could handle that in __init__, but it would have to be explicit code. This would prevent creating the component and then later setting its ID, but not sure if that's really a pattern we want to promote.
seems current chrome shifts something half a pixel?
also give set_random_id a leading underscore so it doesn't
need to become a reserved word (disallowed prop name)
sys.modulesthough, that kind of scares me...)CHANGELOG.mdcc @chriddyp @emmeb seem like reasonable error messages? The traceback will point to the callback definition, and users CAN just set the component ID right there though probably better is to follow the variable reference back to where the component is first created and set the ID at that point.