Convert to ESM

[fgblomqvist]

ESM is the new black; CommonJS can be safely left in the past for projects that aren't meant to be imported (like this one).

Performing this conversion would aid an eventual TS conversion (#118) as well, though, it's worth noting that switching to ESM should be done regardless, to stay with the times.

The easiest way to do it without blocking other work too much is to go file-by-file by renaming the file to .mjs, and changing all the imports of it and in it. You need to start at the "top" of the import chain and work your way down since a CJS file cannot import an ESM file. Once all the files in the repo have been converted you can set package: "module" in the package.json and then do a bulk-rename back to .js for all files.

[fgblomqvist]

I'll open a PR with some initial conversions as a poc

[simonv3]

Womp womp, looks like this won't quite work for mocha yet?

https://mochajs.org/#current-limitations

Watch mode does not support ES Module test files

[fgblomqvist]

Sounds like there's some ways to work around it:
mochajs/mocha#4374

Running with the --parallel flag, or using nodemon. Maybe give that a shot? I can try playing around with it a bit later today

[fgblomqvist]

Tried playing around with it but got stuck with getting the tests to run properly at all (the test command seems to hang after it finishes? Maybe related to the docker setup), will report back once I've figured it out.

EDIT: Just found the test readme, completely overlooked that one (maybe it should be linked from the root one)

[fgblomqvist]

I managed to get everything running by following the readme, but I am not able to produce the error with the watch mode that people are talking about. Though, running with nodemon also works fine on my end (and as detailed in the issue thread) so there shouldn't be any issues anyway. Have you been able to trigger the error?

As a side note, seems like there is a slight instability in some tests (where they some times pass and sometimes fail, but I'm guessing that's just apart of the WIP stuff).

[simonv3]

As a side note, seems like there is a slight instability in some tests (where they some times pass and sometimes fail, but I'm guessing that's just apart of the WIP stuff).

Could you keep track of which tests you're seeing this instability in? I haven't noticed any myself. Maybe in #59? Edit: I think there's at least an ordering issue going on (cause IIRC postgres doesn't guarantee order) with plays/resolve. I think I've fixed that locally.

I managed to get everything running by following the readme, but I am not able to produce the error with the watch mode that people are talking about. Though, running with nodemon also works fine on my end (and as detailed in the issue thread) so there shouldn't be any issues anyway. Have you been able to trigger the error?

IIRC it wasn't running the tests at all, but it's been a minute. I can hopefully try again this weekend.

[fgblomqvist]

I think there's at least an ordering issue going on (cause IIRC postgres doesn't guarantee order) with plays/resolve

Yeah one test that was failing every now and then was UserPlays.test.js > "should GET /user/plays/resolve". The play count went between 1 and 2 (expecting 2) on line 129. But yeah nice that you might have fixed it.