Using Avivator as a library

[xinaesthete]

User story

In MDV, we support loading arbitrary sets of charts, including Viv based views. I'm gradually refactoring some of our code (particularly Viv-related parts), moving more towards React and it'd be nice to be able to re-use some Avivator components, as well as the hooks and state.

Preferred solution

Zustand stores can be adapted to use React Context API. In Avivator, there can be one provider for the whole app which doesn't appear to impact on functionality AFAICT.

I posted this a couple of weeks ago, and will submit a PR referencing this issue based on my branch.

Possible alternatives

I really don't want to have to keep maintaining a fork, or re-implement my own version of equivalent code.

Unfortunately I don't think there's a way to do this that doesn't touch all parts of the code where useXxxStore.setState() is used, but would be happy if someone could correct me.

[xinaesthete]

Excuse my less-than-stellar git skills - I merged recent changes where I should probably have rebased, so there is currently a lot more noise than I'd like in my draft PR. Hopefully I'll manage to make that a bit cleaner.

[ilan-gold]

We will have a discussion about this. We have been not in favor of this in the past for the reasons you alluded to (more docs, responsibility, etc.) but perhaps there is some middle ground. Thanks!

[xinaesthete]

I hope there's a middle-ground, because I think it'll be a bit of a nuisance for us otherwise I'm afraid.

I appreciate that when there is only one context this ends up making the code marginally less clean, and it's unfortunate that it ends up leaking into wider parts of the codebase... the main problem with that as far as I'm concerned is when it comes to merging your upstream changes. Great to see some active development, but really hoping not to have to keep doing this...

There are a few things I want to potentially change later with shaders etc that may hopefully be of some benefit to you upstream... but I may end up having to make an Avivator fork and none of us want that. Shader things are probably mostly at the Viv layer (although I have some experimental shader editing Avivator UI in another branch - not in a state that I would suggest would be suitable for the main app at all, although I may want to develop it further into something that could be made more widely useful).

Anyway, thanks for your response. I could be willing to contribute in terms of documentation if it helps - not sure how bad it would be though to just have a heavy disclaimer on this not being something you want to support, in which case I don't think it'd make for any particular extra maintenance on your part.

[ilan-gold]

I think shader changes are much more likely to be accepted since I am the one who has responsibility over them. But making a changing like promising some API to end-users would be a lot for Avivator. What is your plan beyond the context business for Avivator? Is that all you would need?

[xinaesthete]

Well, what I want to be able to do is npm install something that lets me import Avivator hooks & probably some of the UI components - I haven't looked into what exactly would need to change in the way the project gets published to facilitate that, but in terms of how Avivator itself is written, all I think I need is the context changes. I'd like some better typing as well, and had some WIP on that which I'd be more than happy to port into my branch for you to review (MDV is messy, sorry for that, but I'm hoping that can be gradually improved especially if we can delegate more code things like viv). I appreciate you probably have your own plans in that regard.

Anyway, so in concrete terms I'd want to be able to pull in a <ChannelController /> for example... Down the line I'll probably want different UI, which I expect will mostly be independent of Avivator, but probably using the hooks etc... some things I'd like to see like histograms along the contrast sliders might be viable for PRs if @manzt is receptive.

As far as avivator-as-a-library is concerned, I'm not worried about it being documented or supported (although hope that if I do run into trouble you'll be able to help - generally I can find my way around ok)... so as I say, I hope that it will be possible to expose what I need with appropriate heavy disclaimers associated and no particular cost in terms of maintenance.

I could potentially make a fork keeping changes to a minimum beyond whatever is needed to expose as a library, it's just unfortunate that the way Zustand works means that the context related changes leak across the code-base making that a chore to keep up-to-date.

[manzt]

Sorry for my limited availability to address this PR right now. The end of the year push is pretty intense for me, and I'm hoping I'll be able to have a look in a couple of weeks to think about this more deeply. I am very open to the PR for using the context to make the use of Avivator in your use case.

Well, what I want to be able to do is npm install something that lets me import Avivator hooks & probably some of the UI components

While I understand this would be ideal for your usage, I don't think we as a team have the capacity to maintain Avivator as a library. I hope that we can find a middle-ground where you are able to find a way to remix pieces of Avivator in your code (hopefully your context PR helps with that) but we are at capacity maintaining Viv's core for library consumers. Exposing Avivator as a package on NPM would be a massage API surface area that we would be taking on long term. There are choices with dependencies in Avivator that we would likely would have made differently if we intended it as a package.

[ilan-gold]

A nice middle ground could be doing things that make installing directly from git easier via npm in some way. I'm not sure what this could look like, but even if it's not encouraged, PRs like the one you already made would be welcome as long as they don't mean taking on an API.

In other words, if you want to get yourself familiar with the Avivator codebase, you're welcome to do that but we can't promise anything about it (besides that the demo will work).

[xinaesthete]

That all seems perfectly reasonable. I'm pretty familiar with the Avivator codebase already, and don't want to incur extra maintenance burden on you... admittedly my own experience of maintaining modules that are actively used more widely is pretty limited/non-existent. Hopefully we'll be able to sort something out that allows for the kind of use I want without putting you on the hook, but I can see there are reasonable grounds for caution.

Interesting point about dependencies in Avivator that I had somewhat pushed out of mind... appreciate that these things can be easy to overlook from outside but then end up being the thin end of a metaphorical wedge... I'm sure it's not the only thing, but I have mixed feelings about Material-UI before, and did vaguely notice the versions you were using being no-longer supported... I can see that being a root of potentially bigger and longer-term headaches than me just doing what I was originally going to do, treating Avivator more as a 'demo' as you say... it may be that there's not that much point in directly re-using much of your UI code in the end, and the state stuff is relatively small & perhaps not worth all this fuss. I just started to get the feeling that with development suddenly starting to be very active on your side, it could be a bit of a hassle to keep doing the same refactors...

So I think there's still some benefit to the context changes - but to be fair there's also an argument that for Avivator itself, they do add a bit of noise to the code...

Anyway, best wishes @manzt & @ilan-gold ... sorry if I'm rambling etc.

[dahannes]

Hey I have just integrated avivator into an existing vite setup and was having quite some issues with dependencies. I needed to downgrade my own zustand and work around material-ui conflicts. I also needed to override the deck.gl imports, but that's probably a different issue.

I would suggest to open a PR with latest versions of both zustand and material-ui. Please let me know if you have any concerns and/or suggestions.

[ilan-gold]

@dahannes Thanks for raising this. We are upgrading deck.gl right now. And I will finish the zustand upgrade as well.

[dahannes]

Thanks for looking into this @ilan-gold !

I just wanted to give this a try, but then I realized that npm has not been updated in 8 months. Also npm lists v0.16.1 as latest, but your package.json shows 0.14.2.

How do you usually go about updating npm? Is it recommended to checkout the github repo?

[ilan-gold]

Is that causing issues? The root-level package.json doesn't actually affect the package version.

[dahannes]

No, the package.json version does not matter. I actually just found this changeset PR #839

The issue is, that I cannot npm install @hms-dbmi/viv to get your updates, right?

[ilan-gold]

No not yet but also (1) Avivator is not released in the package and (2) I haven't done material-ui yet. But the zustand stuff has been merged now