Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Validation on component properties #264

Closed
chriddyp opened this issue Jun 8, 2018 · 4 comments
Closed

Validation on component properties #264

chriddyp opened this issue Jun 8, 2018 · 4 comments
Assignees

Comments

@chriddyp
Copy link
Member

chriddyp commented Jun 8, 2018

This is part of the "A Pleasant and Productive Developer Experience" objective ❤️

Currently, we don't do any validation on property values of components. So, if you pass in

html.Div([[]])

instead of

html.Div([])

Dash will not raise an error but instead silently fail in the front-end, sometimes but not always showing the notorious "Error loading dependencies" error message.

We have all of the prop-type values available in the component's metadata.json files, extracted from the component's propTypes.
For example, here is the Dropdown.react.js prop types:
https://github.com/plotly/dash-core-components/blob/7c2b102c680d9fe0cb9df61f1682feccfa3ddc05/src/components/Dropdown.react.js#L101-L180
and here is it's associated JSON types:
https://github.com/plotly/dash-core-components/blob/7c2b102c680d9fe0cb9df61f1682feccfa3ddc05/dash_core_components/metadata.json#L810-L946

So, implementing prop type checking would involve:

  • Making sure that all component props are sufficiently described. I'm sure that flexible properties like children aren't sufficiently described. For example, children is most explicitly:
propTypes.oneOfType([
    PropTypes.string,
    PropTypes.null,
    PropTypes.number,
    PropTypes.node,
    PropTypes.arrayOf(
        propTypes.oneOfType([
            PropTypes.string,
            PropTypes.null,
            PropTypes.number,
            PropTypes.node,
        ])
    )
])
@zogzog
Copy link

zogzog commented Oct 31, 2018

Well, ok. However the experience of working dash apps suddenly not starting was not that pleasant (I know, I should have pinned a precise dash version).
Also I had to stuff children={} all over the place. And replace a few Nones with ''. That gets the validator satisfied. And then "Error loading dependencies".

@rmarren1
Copy link
Contributor

rmarren1 commented Nov 2, 2018

@zogzog Thanks for taking the time to check out this PR! This is still packaged as a release candidate, so it shouldn't have been installed whether you pinned the version or not unless you explicitly asked for it (e.g. pip install dash==0.29.0rc7). Did you install it using something different than pip?

I'd be interested to know the specific components that required children={} or <something>="". Do you happen to remember which ones these were? The former should not have been required (perhaps a bug on our end, I do not think any components define children as a JSON object), the latter is something would happen often since some string props are not nullable.

Also, do you have an example that produces the "Error loading dependencies" bug? This is a common error in Dash when component properties are the wrong type, which would be bad because that's what this PR should solve. I think it may be children={} causing this.

@zogzog
Copy link

zogzog commented Nov 9, 2018

@rmarren1 I had (mistakenly) pinned everything but dash itself and I hit this the last time I attempted a deployment. So as I previously reported I did whatever I could to satisfy the validator, only to land on the "error loading dependencies" thing. In the end I wound up pinning dash to 0.22.0rc2 to have a working version.

I think the validation thing is a good idea. However the js side of things is a bit creepy and debugging these errors remains unspeakable. Obviously since April things have moved a bit and even with a validated html structure I get into a non-working system.

As for an "example" I can only point to the relevant components : https://bitbucket.org/pythonian/tsview and https://bitbucket.org/pythonian/tshistory_editor ... (they're small components and can easily be reviewed. If you have look, I apologize in advance for the suboptimal formatting of the html part).

I fear I have thrown away (in anger) the commits that made validation happy ... (though some vcs surgery might bring them back to life, but not before monday :).

@chriddyp
Copy link
Member Author

Finished in plotly/dash-renderer#100

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants