-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
187 support flow types #207
Conversation
…-support-flow-types
very nice! will take a look at this this week. do you have an example of a Dash react component that uses Flow? am curious about what the user would need to modify after generating the boilerplate from the |
I don't believe a user would need to modify anything if you use the current boilerplate. The babel react preset already supports stripping Flow types. You would Here's a As a side note, pylint is very aggressive and not playing nice with my PEP8 linting. I think I finally got it where the tests pass. |
@chriddyp Need anything else on this? Also ran the codemods to help out with optionally updating the bundled React version for |
"'{}'".format(t) | ||
for t in list(type_object['value'].keys())), | ||
'Those keys have the following types: \n{}'.format( | ||
'\n'.join(create_prop_docstring( |
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.
🙇 🙇 🙇 🙇
@mjclawar and @coralvanda - Sorry for the delay on this one. I've finally found some time to review and this looks good to me. Many, many thanks for writing the thorough tests and taking the time to improve the existing code. Is there anything else that you would like to add? Otherwise, I'll merge this tonight and make a new release. I think that I'd like to make a separate repo containing a sample flowtypes component that I can include across Dash's end-to-end integration tests. I can do this separately and it shouldn't block this PR. |
I created this here: https://github.com/plotly/dash-flow-example. I'll add an integration test that includes these prop types once this PR is merged. Screenshot of the example app: @mjclawar or @coralvanda - How would I convert these nested
I'd like to add a couple of nested properties to the flow example: https://github.com/plotly/dash-flow-example/blob/master/src/components/ExampleFlowComponent.react.js#L3 |
@chriddyp nice!
is equivalent to {
/** A list of options*/
options: Array<{
/** The label of the option */
label: string | number,
/** The value of the option*/
value: string,
}>,
/** A config object */
config: {
/** The title option*/
title: string | number,
/** The value of the option */
value: string,
} You also can create a temporary type object to use elsewhere throughout the code, which is the best part! Example: type T_OPTION = {
/** The label of the option */
label: string | number,
/** The value of the option*/
value: string,
};
type T_CONFIG = {
/** The title option */
title: string | number,
/** The value of the option */
value: string
};
...
{
/** A list of options */
options: Array<T_OPTION>,
/** A config object */
config: T_CONFIG,
} Once this is updated, more than willing to pass along a couple of examples from our libraries for more Flow examples. |
Nothing besides wishing there were also typescript support for completeness, but I have never used that and have no interest in learning tonight! I think this PR completely addresses the issue. Plenty of examples can follow in other repos, and I would guess that the https://plot.ly/dash/plugins page could benefit from some updates after this. |
Great! Nice work @mjclawar and @coralvanda 🍻 |
I'm going to wait until #212 passes tests before making the formal release. |
🤞 |
* add flowtype integration test sanity check for #207 * white space * add screenshot test to aborted example
so far so good! available in |
@chriddyp see updated e.g., type Props = {
/** Checkbox is checked if true */
checked?: boolean,
/** Checkbox is disabled if true */
disabled?: boolean,
/** A callback for firing events to dash */
fireEvent?: () => void,
/** Overrides the inline-styles of the icon element */
iconStyle?: Object,
/** The element's ID */
id: string,
/** Overrides the inline styles of the input element */
inputStyle?: Object,
/** The text label for the checkbox */
label?: string,
/** Where the label will be placed next to the checkbox */
labelPosition?: 'left' | 'right',
/** Overrides the inline styles of the Checkbox element label */
labelStyle?: Object,
/** Dash callback to update props on the server */
setProps?: () => void,
/** Override the inline styles of the root element */
style?: Object,
}; and type Props = {
/** Label for the action on the snackbar */
action?: Node,
/**
* The number of milliseconds to wait before automatically dismissing. If no value is specified
* the snackbar will dismiss normally. If a value is provided the snackbar can still be dismissed
* normally. If a snackbar is dismissed before the timer expires, the timer will be cleared.
*/
autoHideDuration?: number,
/** Override the inline styles of the body element */
bodyStyle?: object,
/** CSS class name of the root element */
className?: string,
/** Override the inline styles of the content element */
contentStyle?: object,
/** Dash event handler for click events */
fireEvent?: () => void,
/** The element's ID */
id: string,
/**
* The message to be displayed.
* (Note: If the message is an element or array, and the Snackbar may re-render while it is
* still open, ensure that the same object remains as the message property if you want to avoid
* the Snackbar hiding and showing again)
*/
message: Node,
/** An integer that represents the number of times that action button has been clicked */
n_clicks?: number,
/** Controls whether the Snackbar is opened or not */
open: boolean,
/** Dash callback to update props on the server */
setProps?: (props: {open?: boolean}) => void,
/** Override the inline styles of the root element */
style?: object,
}; |
* add flowtype integration test sanity check for plotly/dash#207 * white space * add screenshot test to aborted example
* add flowtype integration test sanity check for plotly/dash#207 * white space * add screenshot test to aborted example
Description
Supports creating docstrings using
react-docgen
for Dash components written solely with Flow type annotations. This is a non-breaking change.The new logic, as determined in discussion in #187 is:
PropTypes
-generated typing, the docstring uses thePropTypes
, regardless of whether the component also has Flow types (current behavior).PropTypes
, the docstring now uses the objects generated byreact-docgen
from the Flow types (new behavior).Implementation
There are no breaking changes, and all pre-existing tests for the
base_component
suite still pass. Unit tests were added by @coralvanda for the docstrings generated by Flow. The unit tests only added anothermetadata.json
file for a Flow-generatedmetadata.json
test example, and aTestFlowMetaDataConversions
TestCase
.As an example, the new Flow support on the Python
dict
in this gist will create this docstring:Thanks to @coralvanda, this now also supports nested object definitions for Flow, e.g. this section:
which is pulled recursively from the
requiredNested
prop found here in the gist.Related issue
Closes #187
Bumps version to 0.20.1, assuming this is appropriate.