Update dependencies & README

[jannis-baum]

Close #69

[jannis-baum]

Hey @tuurep this would be ready to review now as well :) This is just dependency & documentation updates. Notable changes that had to be made due to the updates were a couple of typing-related things and the switch to ESLint's new flat config.

(I already added the "we support full extended syntax" claim to the README to clean it up; I think it's okay, we will have it before the next release)

[tuurep]

I have to get back to this soon

But can you tell me the command to run the lint?

Tried:

$ yarn lint
yarn run v1.22.22
$ eslint src static
/bin/sh: line 1: eslint: command not found
error Command failed with exit code 127.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Then after installing package eslint from pacman repos:

$ yarn lint
yarn run v1.22.22
$ eslint src static

Oops! Something went wrong! :(

ESLint: 9.7.0

Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@typescript-eslint/parser' imported from /home/tuure/projects/vivify/eslint.config.mjs
    at packageResolve (node:internal/modules/esm/resolve:842:9)
    at moduleResolve (node:internal/modules/esm/resolve:915:18)
    at defaultResolve (node:internal/modules/esm/resolve:1132:11)
    at ModuleLoader.defaultResolve (node:internal/modules/esm/loader:557:12)
    at ModuleLoader.resolve (node:internal/modules/esm/loader:526:25)
    at ModuleLoader.getModuleJob (node:internal/modules/esm/loader:249:38)
    at ModuleJob._link (node:internal/modules/esm/module_job:126:49)
error Command failed with exit code 2.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

[jannis-baum]

But can you tell me the command to run the lint?

Should be just yarn lint. Did you run just yarn before? This is necessary now because lots of versions changed so they have to be installed

Then after installing package eslint from pacman repos:

This is not necessary :)

[tuurep]

Ahh, right!

Most likely that's it. I'll be back.

[tuurep]

There's one typo in CONTRIBUTING but other than that, I see no issues

[tuurep]

As far as dependencies bump, I made sure that yarn or make linux don't spit any new errors and such - all works!

Can't test at runtime because of the Node SEA issue of course :) but I'm gonna check the nvm etc. thing next so we'll see.

Edit:

I could test with the CI build artifact of course - no issues.

But now I notice that when unzipping the artifact, viv and vivify-server don't have executable bit set, when they previously did. I don't know if this is intentional or if it matters, but I'll point this out 😄

[tuurep]

Not a change in this PR but relevant to the README: the double ! is a bit startling xD But I assume that's a typo.

[jannis-baum]

But now I notice that when unzipping the artifact, viv and vivify-server don't have executable bit set, when they previously did. I don't know if this is intentional or if it matters, but I'll point this out 😄

Oh, that's annoying haha. What's worse is that it seems this has been an open issue on the GitHub-maintained upload/download-artifact actions for almost five years! The accidental double-zipping we had before actually protected us from this. Not sure what to do here. Seems like double-archiving is actually the only way to work around it. We could e.g. also tar manually and then the action would zip it.

What do you think, double archive or wrong permissions? I think I am leaning towards double archive to be honest, seems like something someone who doesn't know much about technical stuff would figure out more easily.

Most important will be to know what the release action will do. Couldn't find clear docs on it. I guess we will just have to see what happens and then bump the releases until we fix it :(

[jannis-baum]

Most important will be to know what the release action will do. Couldn't find clear docs on it. I guess we will just have to see what happens and then bump the releases until we fix it :(

Okay so I ended up making a test repo to figure this out. I took like 10 tries until it worked so I think it's good that didn't happen here haha. It's super hard to test, but this is what worked in the end on the test repo where linux/hehe.sh is an executable:

jobs:
  build-linux:
    name: Build Linux
    runs-on: ubuntu-latest
    steps:
      - name: checkout
        uses: actions/checkout@v4
      - name: upload artifact
        uses: actions/upload-artifact@v4
        with:
          name: vivify-linux
          path: linux

  release:
    name: Release
    needs: [build-linux]
    runs-on: ubuntu-latest
    steps:
      - name: download linux artifact
        if: startsWith(github.ref, 'refs/tags/v')
        uses: actions/download-artifact@v4
        with:
          name: vivify-linux
          path: ./linux
      - name: archive
        run: |
          ls -la linux
          chmod +x linux/hehe.sh
          zip -r linux.zip linux
      - name: release
        if: startsWith(github.ref, 'refs/tags/v')
        uses: softprops/action-gh-release@v2
        with:
          generate_release_notes: true
          files: '*.zip'

Up/download artifact seems to flatten the file hierarchy and download unzips automatically just as upload zips. So in the release job we end up with just the files, but with stripped permissions, at the specified path. What I added then was to redo the permissions and zip before releasing because the release action does not archive anything automatically. Fingers crossed that I transferred this to Vivify correctly and that the release will work. Maybe you can check if the changes from 0051013 make sense with regard to the working example above?

What do you think, double archive or wrong permissions? I think I am leaning towards double archive to be honest, seems like something someone who doesn't know much about technical stuff would figure out more easily.

In regard to this I would actually argue now to just leave it as it is because the person without technical expertise won't need to directly use the CI outputs anyways. Let me know what you think!

[tuurep]

Ok!

I was definitely wondering if there's a difference between a release and a build artifact, so that clears that up. Although unfortunate that the release is so hard to test.

So... I guess I approve but with fingers crossed as well 😄

Unless, would I be able to test how your test repo release zip behaves on my system?

[tuurep]

In regard to this I would actually argue now to just leave it as it is because the person without technical expertise won't need to directly use the CI outputs anyways.

And yes, this isn't so important, as long as it doesn't affect the release or anything else. Me downloading these artifacts will be temporary if all goes well + I can always script this workflow for these tests.

[jannis-baum]

Thanks a lot!

So... I guess I approve but with fingers crossed as well 😄

Unless, would I be able to test how your test repo release zip behaves on my system?

I invited you to the test repo if you want to have a look. I tried to make a minimal but full replication of what should happen on Vivify there. Feel free to do whatever you want there haha, e.g. make more releases (./release minor) and stuff. But just if you want to, if not we can just merge this and keep the fingers crossed. Let me know :)

[tuurep]

Ok, thanks! I tested that the release unzips sensibly and hehe.sh is executable, all works there.

I'll check one thing: whether build artifact here has the 'can't open multiple viewers' issue, or if it's introduced in the ESM PR

[jannis-baum]

Ok, thanks! I tested that the release unzips sensibly and hehe.sh is executable, all works there.

Nice, thanks! And does Vivify's CI look like it does the same thing to you?🙈

[tuurep]

I'll check one thing: whether build artifact here has the 'can't open multiple viewers' issue, or if it's introduced in the ESM PR

This doesn't happen here, confirmed! And I double checked that it's indeed the case that this is happening in the ESM PR. I'll start debugging as you suggested and lets continue on the other PR

Nice, thanks! And does Vivify's CI look like it does the same thing to you?🙈

Hmm if it means this part:

run: |
          chmod +x ./vivify-linux/* ./vivify-macos/*
          zip -r vivify-linux.zip vivify-linux
          zip -r vivify-macos.zip vivify-macos

then sure, looks like the same

I think this PR is ready to merge, unless anything else :D