dash icon indicating copy to clipboard operation
dash copied to clipboard

Fix dependencies between Dash components

Open pikhovkin opened this issue 2 years ago • 6 comments

I have a dash-component1 that depends on react-lib1. I also have dash-component2, which depends on react-lib1 and dash-component1. dash-component2 is a plugin to dash-component1. Since ComponentRegistry.registry is an unordered set, the Dash components added to it may not be in the order in which they were added to it. I need to load scripts (dash components) in the order in which they were imported. I suggest a small change in dash/development/base_component.py

...

class OrderedSet(collections.OrderedDict):
    def add(self, item):
        self[item] = None


class ComponentRegistry:
    registry = OrderedSet()  # set() <<
...

pikhovkin avatar Jun 23 '22 18:06 pikhovkin

Thanks @pikhovkin! Just to be sure I understand the situation, you're trying to ensure that the files in _js_dist for dash-component1 are included on the page before the files in _js_dist for dash-component2, so that dash-component2 can use items loaded by dash-component1 during script execution? This might be avoidable if the dependency were moved into component initialization instead of script execution, but that might not always be feasible.

Depending on import order feels a bit fragile to me, but I suppose you can have dash-component2 itself import dash-component1 in its __init__.py, so that this doesn't depend on the user getting the order right, is that what you intend to do? I guess if we document this as the "official" way to have one component depend on another, that would cover the case where the first package is required to use the second. There might be another use case of two packages both independently needing the same resource and you only want to load it once; perhaps we can ignore that case for now.

I think this would be better with https://pypi.org/project/ordered-set/ rather than using OrderedDict as a stand-in, just in case users are depending on ComponentRegistry.registry having set behavior that the Dash core itself doesn't use.

alexcjohnson avatar Jun 27 '22 18:06 alexcjohnson

@alexcjohnson thanks for the answer, but you misunderstood. Regardless of where and when the dependencies are imported, the order of those dependencies depends on the unordered ComponentRegistry.registry, which does not preserve the order of the elements. Try run

>>> s = set(['h'])
>>> s.add('a')
>>> s
{'a', 'h'}
>>> s.add('g')
>>> s
{'a', 'h', 'g'}

I think this would be better with https://pypi.org/project/ordered-set/ rather than using OrderedDict as a stand-in, just in case users are depending on ComponentRegistry.registry having set behavior that the Dash core itself doesn't use.

You can use third-party libraries if you like. Although only the add method is used in the ComponentRegistry.registry. My microvariant works fine. And you can always use a third-party library later. And also https://pypi.org/project/ordered-set uses the dict internally. Why pay more? :-)

pikhovkin avatar Jun 27 '22 18:06 pikhovkin

which does not preserve the order of the elements

I understand that, I'm just trying to pin down WHY I would care about that order. Most of the time import order is irrelevant, as is the case for Python and JS imports in general. There are conventions about what order you should put imports at the top of a module, but it's supposed to be just conventions. If the behavior is different when you change that order it would generally be considered a bug. Did I understand correctly that what matters to you is the order the JS scripts are executed on the page? And that (after we make ComponentRegistry.registry preserve order) having your dash-component2 import dash-component1 internally would hide this order dependency from your users?

Although only the add method is used in the ComponentRegistry.registry

Right, that's what I meant by "behavior that the Dash core itself doesn't use." What I'm concerned about is other users of Dash that may have found a reason to inspect ComponentRegistry.registry in their own code and therefore depend on it having other set behavior. I'm not aware of any such uses, but Dash has lots of very clever users and we don't want to break their code :)

alexcjohnson avatar Jun 27 '22 20:06 alexcjohnson

I think this would be better with https://pypi.org/project/ordered-set/ rather than using OrderedDict as a stand-in, just in case users are depending on ComponentRegistry.registry having set behavior that the Dash core itself doesn't use.

I also think it would be best if this was a proper set.

T4rk1n avatar Jun 28 '22 13:06 T4rk1n

I'm just trying to pin down WHY I would care about that order

The order is important when the plugin should work. The plugin depends on the parent component (a dash-my-plugin-component depends on a dash-third-party-component). In my case, the plugin needs to get the parent context. When I say a "plugin" is a plugin, it wraps a react component that needs a parent context. That is why the order in which scripts are loaded is important here. All Dash components that I have seen contain all the features in themselves, in their internal scripts. Therefore, they can be imported in any order. The script loading order for a Dash component will be provided by _js_dist. But there are cases when you need to develop components with more complex dependencies, where the script loading order is important, where the parent component must be imported before the plugin. Here is just such a case.

I'm not aware of any such uses, but Dash has lots of very clever users and we don't want to break their code

Have you ever seen an example of someone doing something with .registry? I'm willing to bet that 99.9(9)% Dash users don't know anything about .registry. .registry is an undocumented feature. The remaining percentage of users are taking a reasonable risk by doing something with .registry other than adding an element. But I don't mind you using a third party package to keep order in .registry.

pikhovkin avatar Jun 28 '22 20:06 pikhovkin

I am also interested in progress on this PR. As I understand, it would enable people to wrap leaflet plugins as separate Dash components, and use them with dash-leaflet. Which would be awesome :)

emilhe avatar Jul 22 '22 11:07 emilhe

@alexcjohnson How can I help to speed up merging of this PR?

pikhovkin avatar Sep 29 '22 09:09 pikhovkin

@pikhovkin as I said before I'm not aware of any external uses of ComponentRegistry.registry, but being undocumented is different from not being public. There are no leading underscores to access it, so it's part of our public API and we should try not to break it.

That being said, there are actually three things Dash depends on this object doing (.add, in and list()), your implementation does all three correctly, and these are the main things I can imagine folks wanting to do if they were to find a use for directly inspecting the registry.

The only other thing I can think of is maybe removing items? Like if package A gets pulled in by package B, so A is implicitly added to the registry, but you don't actually want package A (and the extra JS it loads on the page). Then you could imagine wanting to remove A from the registry. Sets have discard and remove methods for this, that dicts don't have. They also have pop with a different signature from dict.pop, though I can't imagine users wanting to use this.

So we could add these methods, or even wait to see if anyone complains and then add them... but to my mind it'd be easier to just use the ordered-set package and be done with it, it's a pretty simple package so a minor cost to pay. Just make sure to allow at least down to v4.0 since Dash still supports Python 3.6 - or don't even restrict its version at all, it's been stable a long time.

alexcjohnson avatar Sep 30 '22 20:09 alexcjohnson