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

xinaesthete · 2024-10-02T13:42:40Z

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 · 2024-10-02T13:51:00Z

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 · 2024-10-02T14:41:03Z

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

it doesn't like single quotes in index.css
<div role="tabpanel" ... in Controller.jsx has an error " ✖ The element with this role can be changed to a DOM element that already this role." - changing to <tabpanel ... seems ok.

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 · 2024-10-02T15:29:00Z

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 · 2024-10-02T15:41:26Z

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

manzt

Thank you for opening a PR for discussion. I apologize if I'm missing some context, but it would be really useful to look at the logs from Vercel. I assume we're using some integration with Vercel on your fork to get previews of the various PRs you're opening.

While that's great, I also want to understand why we need to make these changes. It would be more useful to investigate your Vercel setup rather than make changes to Viv, especially since this isn't something we need to host.

I'm not an avid user of Vercel, but I understand they are very oriented around NextJS apps. Their "out of the box" setup tries to do some magic with installations and build steps. The purpose of devDependencies is to separate what's required in the website from what's required to build the website. This is useful for maintainers to understand the relationships between dependencies.

With Vercel, my understanding is that they assume you're making a NextJS app, and therefore they will "skip" devDependencies since they know what's typically required to build NextJS apps. Their default behavior is generally to only install production dependencies.

Have you specified a custom install command in your Vercel setup? I think pnpm install should do the trick. My guess is the default behavior by Vercel is to exclude development dependencies since they know the toolchain for NextJS apps.

manzt · 2024-10-02T15:42:40Z

package.json

@@ -34,7 +34,7 @@
 "@svitejs/changesets-changelog-github-compact": "^1.1.0",
 "esbuild": "^0.19.5",
 "esno": "^4.0.0",
- "gl": "^6.0.2",
+ "gl": "^8.0.2",


was this causing an issue in Vercel or for you locally?

locally - couldn't pnpm i without this

xinaesthete · 2024-10-02T15:43:33Z

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 · 2024-10-02T15:48:18Z

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 · 2024-10-02T15:53:50Z

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

xinaesthete · 2024-10-02T15:56:00Z

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 · 2024-10-02T15:59:24Z

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 · 2024-10-02T16:47:15Z

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:

Also make sure the Root Directory is blank.

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

manzt · 2024-10-02T18:12:09Z

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

xinaesthete · 2024-10-03T12:51:58Z

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

manzt · 2024-10-03T13:30:33Z

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

xinaesthete added 2 commits October 2, 2024 14:36

(fix) update "gl" so "pnpm i" works (on mac, maybe relevant)

20a6f36

(fix) move dependencies needed for prod out of devDependencies

b605672

xinaesthete changed the title ~~changes that seem necessary for main to work~~ changes to devDependencies that seem necessary for main to work Oct 2, 2024

xinaesthete force-pushed the avivator-vercel branch from 9e78242 to b605672 Compare October 2, 2024 15:37

Merge remote-tracking branch 'origin/main' into avivator-vercel

1ec3c43

manzt reviewed Oct 2, 2024

View reviewed changes

manzt mentioned this pull request Oct 2, 2024

chore: Remove gl devDependency #829

Merged

manzt closed this Oct 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

xinaesthete commented Oct 2, 2024

xinaesthete commented Oct 2, 2024

xinaesthete commented Oct 2, 2024

manzt commented Oct 2, 2024 •

edited

Loading

xinaesthete commented Oct 2, 2024

manzt left a comment

manzt Oct 2, 2024

xinaesthete Oct 2, 2024

xinaesthete commented Oct 2, 2024 •

edited

Loading

manzt commented Oct 2, 2024 •

edited

Loading

manzt commented Oct 2, 2024 •

edited

Loading

xinaesthete commented Oct 2, 2024

manzt commented Oct 2, 2024 •

edited

Loading

manzt commented Oct 2, 2024 •

edited

Loading

manzt commented Oct 2, 2024

xinaesthete commented Oct 3, 2024

manzt commented Oct 3, 2024

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

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

Conversation

xinaesthete commented Oct 2, 2024

xinaesthete commented Oct 2, 2024

xinaesthete commented Oct 2, 2024

manzt commented Oct 2, 2024 • edited Loading

xinaesthete commented Oct 2, 2024

manzt left a comment

Choose a reason for hiding this comment

manzt Oct 2, 2024

Choose a reason for hiding this comment

xinaesthete Oct 2, 2024

Choose a reason for hiding this comment

xinaesthete commented Oct 2, 2024 • edited Loading

manzt commented Oct 2, 2024 • edited Loading

manzt commented Oct 2, 2024 • edited Loading

xinaesthete commented Oct 2, 2024

manzt commented Oct 2, 2024 • edited Loading

manzt commented Oct 2, 2024 • edited Loading

manzt commented Oct 2, 2024

xinaesthete commented Oct 3, 2024

manzt commented Oct 3, 2024

manzt commented Oct 2, 2024 •

edited

Loading

xinaesthete commented Oct 2, 2024 •

edited

Loading

manzt commented Oct 2, 2024 •

edited

Loading

manzt commented Oct 2, 2024 •

edited

Loading

manzt commented Oct 2, 2024 •

edited

Loading

manzt commented Oct 2, 2024 •

edited

Loading