j-holub / Node-MPV

A NodeJs Module for MPV Player
https://www.npmjs.com/package/node-mpv
MIT License
116 stars 73 forks source link

When will version 2 be available on npm? #39

Closed depuits closed 4 years ago

depuits commented 6 years ago

Will version 2 become available on npm or should we should use the git link for our dependencies?

j-holub commented 6 years ago

Hey there,

Version 2 will become available eventually when I feel like it's ready. My current To-Do is to have proper Error propagation. Unfortunately university doesn't leave me too much time to work on it.

For now link to the git, but be aware that everything's suspect to change and I might break things along the way. I wanna make sure that when I release Version 2 it works really well and is in a state that I feel comfortable with.

depuits commented 6 years ago

Thanks for the fast reply. I mostly need version 2 for the promise return of load, but its currently working without so I'll wait for the stable version.

j-holub commented 6 years ago

Locally I have a version where I promisified the internal send function to communicates between MPV and the module which results in every single method returning a promise. This makes the whole module a lot more robust.

The problem is, that the error messages MPV returns over the IPC socket (if it returns any at all) are not very informative. So my problem right now is to figure out how to give the user of the module precise error messages when something went wrong. Unfortunately this turned out to be harder than I originally expected it to be.

j-holub commented 6 years ago

I will pretty soon (maybe even today) push a lot of changes to the Version 2 Branch. This will make every method return a Promise (except isRunning()).

While this also pushes things closer to officially release it also breaks the API a little again. Just keep that in mind. I will make sure that everything is as I like it before I officially release it because then I don't want to introduce breaking API changes.

@AxelTerizaki you are using Version 2 currently with your Mugen Karaoke Project, just thought I'll warn you in case you pull the latest changes

AxelTerizaki commented 6 years ago

Thanks for the heads up :)

I realize this is my mistake in the first place, I shouldn't have used the branch version in the previous Karaoke Mugen releases, as it'll break things when you'll be removing that branch or breaking things.

This is not a big issue though as most people use a packaged version downloaded on the website, so they won't suffer from the breakage.

It'll only be a problem for those still using an old version of KM on a git install, but they'll have to git pull to get to the latest version and it'll all work out in the end.

I'm eagerly waiting for the commits then :) Since I'm using async/await in my code it won't be too much of a problem.

j-holub commented 6 years ago

I was also just thinking if I could use async and await internally within Node-MPV but I don't know really much about it and my first attempts just failed :D

AxelTerizaki commented 6 years ago

async/await is a painless to use, but it's only usable by default (without any flags) since Node 7.6

Using it in node-mpv would keep people on older node versions from using it. You can always use Babel or something similar to transpile the code for the correct Node version but I think you're just going to add complexity to your code. It's not worth it in my opinion.

If breaking compatibility with older nodes (Node 6 is already in EOL support mode after all) is not a big issue to you for thie version 2, then you could use async/await.

If you'd like some help with async/await, we could always talk about it in private, I'd be glad to help you out.

j-holub commented 6 years ago

Thanks for the very informative response. Yeah I think the WebDev world is not really a world where people stick to old version for that long so I don't see breaking compatibility with Node6 as that much of an issue.

But even without using asyc/await internally it already makes calling the API a lot easier

mpv.load('somefile.mp3')
.then(() => {
    mpv.mute();
})
.then(() => {
   mpv.getDuration()
})
.then((duration) => {
   console.log(duration)
}
.catch((error)=> {
    console.log(error);
});

becomes

try{
    await mpv.load('somefile.mp3');
    await mpv.mute();
    console.log(await mpv.getDuration());
}
catch(err) {
    console.log(error);
}

Given that it runs inside an async function.

Kind of nicer I think. As far is this goes, it's only a matter of writing the documentation differently. I'd have to play around a bit with it to see if I could just make every Nove-MPV method async and let it use await internally.

At the moment the methods look like this

function getDuration() {
    return this.getProperty('duration'); // this returns a promise
}

But maybe it could become

const getDuration = async () => {
    return await this.getProperty('duration');
}

So that it could be used like this

const duration = mpv.getDuration()
j-holub commented 6 years ago

Just pushed everything into the Version 2 branch. As always, everything is subject to change as long as it's not in master.

vecernik commented 6 years ago

do you plan to add @types?

j-holub commented 6 years ago

You are referring to TypeScript right? I was actually thinking about it at some point.

I've never done this before though, but it shouldn't be too much of a hassle I think

j-holub commented 6 years ago

I just did a small field text by writing a quick and dirty music server application that plays some stuff from YouTube.

For the most part version 2 works like a charm and I love the new robustness. But I've also encountered some bugs I want to fix before I publicly release it.

Feel free to use the latest version and report back if you find some issues :)

BenjaminLawson commented 5 years ago

Version 2 is working great for me on Node v10.15.3 and macOS 10.14.3! Thank you!

gabrielstuff commented 5 years ago

hello @00SteinsGate00 thanks for the V2 which works perfectly fine and fix the https://github.com/00SteinsGate00/Node-MPV/issues/34

Do you have a list of bugs or features we could tacle to release this v2 ?

Thanks ! HAve a great weekend.

j-holub commented 4 years ago

Hey @gabrielstuff, I know it's been a while. I don't know if you're still using this library at all but the issue tracker is the best place to tackle bugs I'd say.

Have a great weekend :)

vecernik commented 4 years ago

What is a reliable way to install node-mpv 2? github:00SteinsGate00/Node-MPV#Node-MPV-2 nor github:j-holub/Node-MPV#Node-MPV-2 work anymore.

AxelTerizaki commented 4 years ago

The branch way should still work, however it's not reliable as npm/yarn won't be able to detect any changes and thus update the module you installed.

You can reference commits directly too, which is another option, but it has the same issue (at least it allows you to make sure you're pulling the right commit). Yarn's cache doesn't handle this very well, for example, and sometimes it will pull a version from its cache locally rather than on github.

I have an npm module with version 2 published but it's several commits behind as @j-holub made quite some changes lately. I could pull those changes and publish a new version but given how active he is lately, I'd rather wait for an official release from h im rathen than me :p. If you don't need all the latest commits youc an npm install node-mpv-km. If @j-holub is okay with it I can pull the latest changes and republish its new version as a temporary solution while the new package arrives. Or he could also release a 2.0.0-beta or something too :)

j-holub commented 4 years ago

I have no idea why the branch would not be working anymore, it's still there. I am still making changes, some of the break the API (most recently I changed statuschangeto status and changed the way it works), but the branch is still there.

As @AxelTerizaki stated, you can reference a certain commit, with that you can have a certain version of the repo that works for you, since I'm still doing API breaking changes from time to time.

I will try to push towards a release, since I finished university and if corona allows it I'll hopefully working as a software developer full time soon and I don't know how much motiviation for software dev in my freetime will be left then. Currently working on a good way to hook into an already running version of mpv in a local branch. It's already working but it generates some problems that I wasn't able to solve yet. When that is done, I will look over the API and see, if that's the way I think it should be and that's probably it.

And error handling is a thing I should look into more, I'm not sure if I'm satisfied with the error object yet, I think I should at least include a stacktrace.

Does NPM have support for beta versions?

gabrielstuff commented 4 years ago

you can publish anything using tags. So basically you can indeed publish a beta : https://medium.com/@kevinkreuzer/publishing-a-beta-or-alpha-version-to-npm-46035b630dd7 or https://coderwall.com/p/l_7acq/publish-beta-versions-of-npm-modules or even more "direct" : https://docs.npmjs.com/adding-dist-tags-to-packages#example

Would be nice to have this v2 comming.

j-holub commented 4 years ago

Hey guys. As of right now, Node-MPV 2 as available as a beta version on NPM. Jus run npm install node-mpv@beta to install it and automatically receive beta updates. Alternatively, you can install the specific version, if you want to freeze it, using npm install node-mpv@2.0.0-beta.0.

Thanks for bearing with me, I hope you like how version 2 turned out.