-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Expose more types for better static typing options #3540
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
Conversation
| ComponentSingleType = typing.Union[str, int, float, Component, None] | ||
| # Renderable node type. | ||
| ComponentType = typing.Union[ | ||
| str, | ||
| int, | ||
| float, | ||
| Component, | ||
| None, | ||
| typing.Sequence[typing.Union[str, int, float, Component, None]], | ||
| ComponentSingleType, | ||
| typing.Sequence[ComponentSingleType], |
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 type is also duplicated on generate components for backward compatibility. The template is here:
dash/dash/development/_py_components_generation.py
Lines 28 to 35 in 1163158
| ComponentType = typing.Union[ | |
| str, | |
| int, | |
| float, | |
| Component, | |
| None, | |
| typing.Sequence[typing.Union[str, int, float, Component, None]], | |
| ] |
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.
I replaced the duplicated code with import from the base_component, is that ok?
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.
No, it's for backward compatibility of generated components in external components libraries, if the library is built with the new imports but used with a previous version of dash it would not work.
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.
Oh, I see. I removed the import and used the new type.
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.
Would it be ok to replace the duplicated code with the import for 4.0?
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.
No, it's really just building the components need to be backward compatible as we don't want to force components libraries to depend on a particular version of dash.
T4rk1n
left a comment
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.
💃
Hi, I have two small improvements to static typing. This is my first contribution, so hopefully I did it right.
Contributor Checklist
ComponentSingleType, representing a singular renderable entity -> it is possible to render a list of these_Wildcardtype public by renaming it toWildcard, and making it anEnum, which I feel better reflects the intent behind it, plays nice with static type-checkers and prevents users from doing funky stuff with it since it is now public (subclassing or instantiating anything other than the tree variants defined by dash)optionals
CHANGELOG.md