hms-dbmi / viv

Library for multiscale visualization of high-resolution multiplexed bioimaging data on the web. Directly renders Zarr and OME-TIFF.
http://avivator.gehlenborglab.org
MIT License
283 stars 46 forks source link

changes to devDependencies that seem necessary for main to work #826

Closed xinaesthete closed 3 weeks ago

xinaesthete commented 3 weeks ago

As of this writing, this is in a draft state - I want to verify that Vercel does indeed fail when changes to avivator dependencies aren't made. Fixes #825

xinaesthete commented 3 weeks ago

https://viv-avivator-hq3v-git-avivator-vercel-xinaesthetes-projects.vercel.app/ LGTM. Pretty sure main in its current state fails without these changes.

I could really do with getting the other PR merged soon as I have deadlines and it's a bit of a blocker.

Cheers, Peter

xinaesthete commented 3 weeks ago

I notice there's also some other CI failing on main with some fairly trivial biome stuff;

Also it doesn't show up in CI logs but I noticed a warning from biome that --apply is deprecated & to use --write instead.

manzt commented 3 weeks ago

I just made #827, which should resolve the biome stuff going forward. Sorry about that.

You should be able to merge in main and revert the fixes for the latest version of biome.

xinaesthete commented 3 weeks ago

@manzt I did a git reset --hard on those commits & merged main.

xinaesthete commented 3 weeks ago

To be honest I think I set the Vercel thing up ages ago and then I wasn't quite clear myself whether it was something I'd done or not.

So no - as of now I hadn't setup a custom install command etc as I somewhat assumed this was more generally relevant to you than it actually was.

manzt commented 3 weeks ago

To be honest I think I set the Vercel thing up ages ago and then I wasn't quite clear myself whether it was something I'd done or not.

No worries about the uncertainty with the Vercel setup. I'm fairly convinced this issue can be fixed with some configuration since we build and deploy Avivator in various platforms (GitHub CI, Netflify) and would prefer not to make changes that change the semantics and readability of our code base.

manzt commented 3 weeks ago

Let me have a look on Vercel now. I'll send a config that works (if I can get it going).

xinaesthete commented 3 weeks ago

To be honest I think I set the Vercel thing up ages ago and then I wasn't quite clear myself whether it was something I'd done or not.

No worries about the uncertainty with the Vercel setup. I'm fairly convinced this issue can be fixed with some configuration since we build and deploy Avivator in various platforms (GitHub CI, Netflify) and would prefer not to make changes that change the semantics and readability of our code base.

Absolutely - I have no desire to interfere, it just seemed like there was something preventing main from working (which I didn't realise was peculiar to my setup).

So it does seem that the gl version was problematic - I have a fairly strong feeling that may be a macos (or maybe Apple Silicon) thing. I think that part should probably be merged, but I don't think you need to change anything to get my vercel thing working (I have a vague recollection that I originally tried to use netlify in a similar way and had some kind of problem leading me to try vercel instead - but this was quite a while ago).

The glsl-colormap in extensions package may warrant a moments thought; I don't know if it should be a devDependency or not.

manzt commented 3 weeks ago

Yah I definitely thing gl is an issue (classic node-gyp!). I'll have a look. I'm guessing we are on a newer version of node now and that's causing an issue.

The glsl-colormap in extensions package may warrant a moments thought; I don't know if it should be a devDependency or not.

Agreed, good catch.

manzt commented 3 weeks ago

Ok I got main working without any changes: https://avivator.vercel.app/

I think gl is a separate issue. This dependency is just used for running tests, and because it's based on onnode-gyp it requires Python to make binaries. This dependency isn't require for building the core libraries and Avivator.

I'm not sure what system vercel builds on, but the binaries for gl aren't pre-built for it (like your machine) and the install fails for various versions I've tried.

The following vercel.json should work (adding pnpm remove gl prefix to your custom install command):

{
  "buildCommand": "pnpm --filter=avivator build",
  "installCommand": "pnpm remove gl && pnpm build",
  "outputDirectory": "sites/avivator/dist"
}

or in the UI under your project settings:

image

Also make sure the Root Directory is blank.

image

Sorry for the workaround, but I'll work on a PR to fix up gl issue.

manzt commented 3 weeks ago

Should be able to remove pnpm remove gl part with #829

xinaesthete commented 3 weeks ago

So I think this PR is probably redundant now and could be closed?

manzt commented 3 weeks ago

Yup! Let me know if you have trouble building avivator.