Replace http-client got with ky-universal

[Haschikeks]

add "ky-universal" as "got" replacement, to improve browser compatibility

Closes #655

[Borewit]

LGTM, @Haschikeks, nice work.

The unit test probably don't run due to the credentials I use on the action to login into MusicBrainz.
I am more then happy to make you a collaborator @Haschikeks which gives you more control over the repository & GitHub actions.

[Borewit]

Just keep the code styke

Suggested change

	import ky, {Options, ResponsePromise} from 'ky-universal';
	import ky, { Options, ResponsePromise } from 'ky-universal';

[Borewit]

Sorry, I did a wrong test, I thought all was working, but I forgot to recompile.

ky-universal is a pure ESM module since version v0.9.0.
That is breaking the tests.

[Borewit]

When downgrading to v0.8.2 downgrading I run into errors in the ky module.

[Haschikeks]

When downgrading to v0.8.2 downgrading I run into errors in the ky module.

This is because the kv v0.26.0 is a pure ESM module as well. Using v0.25.1 works on my end. It also runs the tests.

[Borewit]

Yet it not really a sustainable solution as these commonjs modules are no longer supported.

[Haschikeks]

I agree. I'll list possible solutions I see for this problem below.
For now, it would help to mention the fact, that the package is a node-only package. (Not everybody reads through open issues)

• Make musicbrainz-api an pure ESM module, which would allow us to use the latest ky packages
• Search for other possible solutions to support browsers, that are not pure ESM packages
• make this package explicitly a node only package without support for running in a browser environment
• expose a fetcher api that the user must provide
  I've seen other repositories do this, I might have to search for an example
  One example where it is possible is rtk-query, where you can provide a custom fetchFn
  • In the Browser this could be the native fetch API
  • In a node environment this could be any package the user likes to use for requests, e.g. node-fetch

[Borewit]

All very good suggestions @Haschikeks.

Make musicbrainz-api an pure ESM module, which would allow us to use the latest ky packages

Pure ESM is nice, but users will complain they cannot get it to work in their CommonJS projects. Yet we choose to do so.

Search for other possible solutions to support browsers, that are not pure ESM packages.

Sure.

Expose a fetcher api that the user must provide

I like that is well. At the same time, not all functions are unfortunately clean REST. Hence we need to work with cookies. This would make wiring a HTTP-client a little cumbersome and tricky.

I not tracking changes in MusicBrainz API at all, could be that they have better implementations nowadays.

Make this package explicitly a node only package without support for running in a browser environment

Essentially close to the "do nothing" option, but document this restriction better.

I have no strong opinion on the direction. If you want to do more work, and you favor any of the above, I will support you.
If you favor ESM, I can apply the same kind of changes as I have done in some other projects. We could also maintain ESM on a different branch, and possible release that as different NPM.

Are you avoiding the question to become a collaborator @Haschikeks?

[Haschikeks]

Are you avoiding the question to become a collaborator @Haschikeks?

Ah, no I'm sorry, I don't try to avoid it. You already offered me to be a collaborator some time ago. I just took my time to accept the invite. But some time ago I accepted it. (At least thats what I think I did? Github shows me as a collaborator in this issue, so)

Regarding how to move on:

Essentially close to the "do nothing" option, but document this restriction better.

For now I would go with this approach until we have a final decisions.

We could also maintain ESM on a different branch, and possible release that as different NPM.

I think we should avoid having 2 packages with the same functionality. But we could think about doing it like ky and have a version be the "deciding" factor for pure ESM module or not.

Expose a fetcher api that the user must provide

I like that is well. At the same time, not all functions are unfortunately clean REST. Hence we need to work with cookies. This would make wiring a HTTP-client a little cumbersome and tricky.
I not tracking changes in MusicBrainz API at all, could be that they have better implementations nowadays.

I can have a look at, how such an interface would need to look like and how we could handle the cookies. I'll also check the MusicBrainz-API in regards to the usage of cookies.

[Borewit]

But some time ago I accepted it. (At least thats what I think I did? Github shows me as a collaborator in this issue, so)

Ah my bad, for some reason I thought you did not. So sorry.

I can have a look at, how such an interface would need to look like and how we could handle the cookies. I'll also check the MusicBrainz-API in regards to the usage of cookies.

It are essentially the functions in which you see formData, which use the non-REST ways of publishing information, which require cookies and CSRF, as they follow the way normal web user would submit the data. Like the function editEntity, addUrlToRecording, addIsrc.