plotly / dash

Data Apps & Dashboards for Python. No JavaScript Required.
https://plotly.com/dash
MIT License
21.15k stars 2.04k forks source link

Allow wildcard props - or arbitrary props - on all components #1021

Open alexcjohnson opened 4 years ago

alexcjohnson commented 4 years ago

Split out from #475 - I'm happy to discuss this issue but don't want to distract from the very different meaning of wildcards in that issue. @ProbonoBonobo said:

For dynamic layout support, should we extend key=id to all core components?

I would encourage it, but let me ask a slightly different question: is there a compelling reason for prohibiting an application from setting arbitrary properties? By "arbitrary properties" I mean any valid html attribute that isn't explicitly defined by a component.

I ask because, in the absence of a common spec, raising a ValueError when an application tries to annotate a node with custom key/value pairs seems awfully perverse. I would understand if that were something that maybe, as web developers, we just learn to live without because transpiling a Python application to React is hard and there's a few tradeoffs. For example, if you try to monkey-patch component instances with a bunch of random props at instantiation time, Dash spits out an error and refuses to build the app until you remove the offending property names.

Why though? If doing so inevitably caused horrible runtime errors, it would make sense that Dash should not only warn but prevent you from shooting yourself in the foot. But when I suppressed those compile-time checks in my local fork (which requires also tweaking the definition of dash.development.base_component.Component.to_plotly_json(), but it's a trivial fix) the app works beautifully. It works exactly as expected. The foreign props are even visible to the dev_tools_ui debugger's callback graph, which was pleasantly surprising.

Originally I had only intended to modify Component.__init__() to add aria- and data- to a component's list of valid prefixes (tangential rant: the base class defers to its subclasses to define _valid_wildcard_prefixes, which is something that's already been codified in the W3C spec. Why? Lots of any third-party components don't define any ._valid_wildcard_prefixes at all, which makes extending them impossible -- and imho they shouldn't have to). But once I saw how to accomplish that, I realized that it would be just easy to abolish the property check completely to support any property. And boom, suddenly I became 2-3x more productive in Dash. It's helpful for so many use cases. Yesterday, for example, I was annoyed that my dbc.Input component applied spellchecking to a name field and didn't expose an option in its signature that to disable that. But since spellcheck is a valid HTML attribute that is parsed by the browser, setting that attribute to "false" on the Python object directly disables the red squigglies where they're unwanted. This would be impossible under the current regime of whitelisted attributes.


If dbc hews close enough to raw HMTL elements that it's obvious how to naturally pass extra props on to them, then I'd encourage you to bring this up over there and add the appropriate extra props (Plotly does not maintain dbc). But in general, dash components aren't simple enough for it to be clear where to pass arbitrary extra props on to - and if we did, they'd often be passed on to another library which in turn would just ignore them. So for the most part this would just be a way of stashing data on the prop, with no direct functional impact. The downside though is that errors can be significantly harder to track down - misspellings, misunderstandings, API changes, all become hunting expeditions rather than simple error messages.

I think we'd be open to providing some sort of standard wildcard prefixes - I suppose this could even be data- and aria- - if we can understand better the value it would bring. But other than the ability to disable prop checks, which you've already discovered, I don't think we want to allow just any arbitrary props. In fact internally we're kind of trying to go in the opposite direction, eg https://github.com/plotly/dash-core-components/issues/703

ghost commented 3 years ago

My team could really use the ability to define data- properties on dcc components; the html components already have data- and aria- options available, this would be a good addition for dcc as well.

As for specific use cases for having this, we are trying to cover our application with Cypress UI tests, and the best practice for selecting elements in tests is by setting a 'data-cy' property on the components, to avoid coupling the test code to the structure and styling of the DOM.

alexcjohnson commented 3 years ago

Digging into this a little, nearly all of the dcc components have a wrapper <div> or other actual DOM element that we could pass data-* props along to. A prerequisite for this would be getting rid of Ramda.omit when we use that to forward props to a 3rd-party library https://github.com/plotly/dash-core-components/issues/703

This isn't on our short-term roadmap, but if anyone would like to make a PR we'd be happy to review and help get it merged.