-
Notifications
You must be signed in to change notification settings - Fork 809
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: youtube base.js parsing logic #1217
Conversation
will check the test cases tomorrow |
@fent please have a look at this PR, and if you agree, please let's merge it :) |
@khlevon Any idea if there are any blockers on merging this? Appreciate the PR! |
@ShantanuNair |
Would you be able to make a version available for me to download and test? |
you can use this version while PR is open: "ytdl-core": "git+ssh://git@github.com:khlevon/node-ytdl-core.git#v4.11.4-patch.2" |
Can you merge this PR ? last versions are broken |
@khlevon the patch is not working anymore, right? |
its still working in my app as of yesterday |
@stedel I'm glad to read that. Mine is getting this error since last Wednesday using the patch mentioned above:
|
Can confirm that this patch does fix #1261 for me. Hoping this gets merged soon! |
* patch-package replaces using git branch for ytdl-core * Need to use postinstall-postinstall because of yarn https://github.com/ds300/patch-package?tab=readme-ov-file#why-use-postinstall-postinstall-with-yarn * Saves ~100MB on Docker image layer by dropping git dependency * Can be removed once fent/node-ytdl-core#1217 is merged
I say if @fent doesn't get back to us in three months, we have someone who knows what they're doing fork this repo and maintain it. There's just a lot of people wanting to contribute that have great ideas, and this repo might die if we don't do anything about it. There just aren't that many great ytdl's out there. |
I second this. |
There already is such a fork: Install this over the old one, update your imports to point at the new |
Summary
Youtube again changed the base.js code for more details, you can check this comment
To have stable lib, I suggest approving this refactoring of
lib/sig.js
file, which was done by @distubejs, and reviewed and improved by me.Results
Fixed issue: #1216
Code tested by me on my own product, also I updated outdated tests.
You can use this patch until we make a decision regarding this PR