smarr / ReBenchDB

ReBenchDB records benchmark results and provides customizable reporting to track and analyze run-time performance of software programs.
MIT License
12 stars 6 forks source link

Project doesn't compile #173

Closed irevoire closed 7 months ago

irevoire commented 7 months ago

Hey, when I run npm run compile on the latest commit of the main branch it throws these errors; image

I don't think it's especially related to my setup or node version, as running docker build . doesn't work either with pretty much the same errors:

Step 20/21 : RUN npm run compile
 ---> Running in f376d9c777c4

> rebenchdb@0.3.0 precompile
> npm run prep-folders && npm run compile-uplot

> rebenchdb@0.3.0 prep-folders
> mkdir -p tmp/interm tmp/knit resources/reports resources/exp-data

> rebenchdb@0.3.0 compile-uplot
> terser --module --ecma 2018 --compress --mangle -o ./resources/uPlot.esm.min.js -- node_modules/uplot/dist/uPlot.esm.js

> rebenchdb@0.3.0 compile
> tsc

src/backend/compare/compare.ts(165,56): error TS2339: Property 'body' does not exist on type 'Request'.
src/backend/compare/compare.ts(230,19): error TS2339: Property 'body' does not exist on type 'Request'.
src/backend/github/github.ts(18,55): error TS2339: Property 'body' does not exist on type 'Request'.
src/backend/rebench/results.ts(31,49): error TS2339: Property 'body' does not exist on type 'Request'.
The command '/bin/sh -c npm run compile' returned a non-zero code: 2
smarr commented 7 months ago

Hi @irevoire, thanks for reaching out.

Have you tried a full npm install .?

I just tried the following on a Ubuntu and macOS, it seems to succeed:

git clone --depth 1 https://github.com/smarr/rebenchdb
cd rebenchdb
npm install .
irevoire commented 7 months ago

Sadly, no, I get the same error as before :pensive: image

I'm running on arch, though, and since there are no dependencies specified, I was lowkey expecting something like that. But I thought it would work with docker build

smarr commented 7 months ago

What version of npm/node are you using?

I am having here npm 9.8.0 + node v20.5.1 and npm 9.9.2 + v19.9.0.

smarr commented 7 months ago

And just to be sure, the installation of the dependencies from package.json/package-lock.json worked without issues?

irevoire commented 7 months ago

What version of npm/node are you using?

Ooh yes, funny, it looks like I got a very outdated version for both npm and node (even though I just installed both of them like one hour ago)

% npm --version
6.14.15
% node --version
v14.18.0

And just to be sure, the installation of the dependencies from package.json/package-lock.json worked without issues?

Honestly, I don't know how anything works in the js/ts ecosystem; I think it worked but cannot guarantee it. The only error I encountered was the one I showed you, and I think the dependencies were gathered at the very beginning of the process, so I would say yes.


After updating npm + node with nvm, it worked, and I was even able to run docker build . thanks! I don't know if there is a way to specify the minimum required version of node or npm, but that would be nice.

And isn't it strange that docker seems to use my npm install instead of using the one we know that works? :thinking:

smarr commented 7 months ago

I don't know if there is a way to specify the minimum required version of node or npm, but that would be nice.

I believe I do exactly that here: https://github.com/smarr/ReBenchDB/blob/master/package.json#L31-L33

But, that might not be backwards compatible and a more recent thing than the versions you got 😄 And in the case of Docker, it's even more odd indeed. The Dockerfile says it is using postgres:16-bookworm, which I would hope implies a certain minimum version. Hm, well, looking at it, Bookworm comes with v18.19, which is lower than what I say I required hmmm...

irevoire commented 7 months ago

I don't know if there is a way to specify the minimum required version of node or npm, but that would be nice.

Looks like there is, and you already did what you were supposed to do here https://github.com/smarr/ReBenchDB/blob/master/package.json#L31-L33

Theoretically, from what I see, the only thing missing is the .npmrc; https://stackoverflow.com/questions/29349684/how-can-i-specify-the-required-node-js-version-in-package-json

But I tried to set it, and it didn't work either in my tests, so I have no idea of what you should do ahah

EDIT: Oops, you were faster than me, but I agree 100% ahah

Bookworm comes with v18.19, which is lower than what I say I required hmmm...

It's super strange that when I updated my local npm version, it fixed the docker build

smarr commented 7 months ago

I spent some time on moving to Docker compose, maybe you could give #175 a try?

irevoire commented 7 months ago

Hey! Yeah, sorry, I'm not at home right now; I'll definitely give it a try on Monday and keep you up

smarr commented 7 months ago

No hurry :)

irevoire commented 7 months ago

The docker-compose did work.

Closing the issue :+1:

smarr commented 7 months ago

Oh, good. Thanks for trying!