Skip to content
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

Upgrade YouTube.js from 4.3 to 5.0.2 (fixes throttling) #3474

Merged
merged 3 commits into from
May 1, 2023

Conversation

absidue
Copy link
Member

@absidue absidue commented Apr 30, 2023

Upgrade YouTube.js from 4.3 to 5.0.2 (fixes throttling)

Pull Request Type

  • Bugfix

Related issue

closes #3457
closes #3475 (dependabot PR)

Description

Upgrades to YouTube.js 5.0.1, as YouTube.js 5 did a bunch of refactoring this PR also has changes for that in it.

The code changes in this PR in no particular order:

  • Update jsdoc types, v5 moved files around so some of them broke, so I decided to go through all of them and use the exported types instead of referencing the paths
  • Use the new endpoint builders and constants (I know that Endpoints.BrowseEndpoint.PATH is verbose but as they are ES6 namespace exports/imports, webpack optimises it to a direct reference to PATH at build time, even for the code inside YouTube.js) for the local API subscription fetching
  • Some properties got renamed, the old names are deprecated and will be removed in a future YouTube.js update, so I switched to the new ones e.g. views -> view_count
  • YouTube.js now parses the dialog in text runs, so we can simplify our code a bit (Fix video comment external link parsing for local API #3448)
  • A change that YouTube.js made a while ago but doesn't seem to have affected us too much so far (only edge cases) is no longer returning "N/A" if a Text element is empty, it now has an Text#isEmpty() method that we can use instead, so I switched to that as well.

Testing

Everything with the local API without video proxying enabled

  1. Check that videos load faster than before
  2. Refresh subscriptions without RSS (if it works it's good)
  3. Test case from Fix video comment external link parsing for local API #3448
  4. Visit the @YouTube channel or https://youtube.com/@YouTube if you prefer a full URL and check that it doesn't spit out errors about not being able to call .replace() on undefined anymore.

Desktop

  • OS: Windows
  • OS Version: 10
  • FreeTube version: 0e11fd0

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) April 30, 2023 11:58
@github-actions github-actions bot added PR: dependencies Pull requests that update a dependency file PR: waiting for review For PRs that are complete, tested, and ready for review labels Apr 30, 2023
@absidue
Copy link
Member Author

absidue commented Apr 30, 2023

The view count on the watch page is currently always NaN, created a PR in YouTube.js to fix it LuanRT/YouTube.js#393.

@PikachuEXE
Copy link
Collaborator

PikachuEXE
PikachuEXE previously approved these changes Apr 30, 2023
@PikachuEXE
Copy link
Collaborator

Screenshot for NaN views
image

@MarmadileManteater
Copy link
Contributor

Most everything looks good. The buffering speed seems to be noticeably improved for me, but I did notice a warning out of youtube.js on the video: https://youtu.be/eBZ2vA9D2RA which didn't seem to be present before. It doesn't look like anything is breaking, but this warning does appear in the console when the video first loads before the comment section is opened, so I don't think this warning has do to with anything that FT uses since the comment section is only shown after an additional request is made for a comment continuation.
image

@ChunkyProgrammer
Copy link
Member

I'm guessing this PR will be updated to 5.0.2?

@absidue absidue changed the title Upgrade YouTube.js from 4.3 to 5.0.1 (fixes throttling) Upgrade YouTube.js from 4.3 to 5.0.2 (fixes throttling) May 1, 2023
@absidue absidue requested a review from PikachuEXE May 1, 2023 10:30
@absidue
Copy link
Member Author

absidue commented May 1, 2023

Views are fixed now.

@absidue
Copy link
Member Author

absidue commented May 1, 2023

@MarmadileManteater that's probably fine, that just means there was a node/renderer in the Innertube response that YouTube.js doesn't know about. Unless something is broken (e.g. missing information, other errors) it's not something we need to worry about, if there is missing information it probably needs to be parsed from the new nodes in YouTube.js.

@FreeTubeBot FreeTubeBot merged commit 170df2a into FreeTubeApp:development May 1, 2023
@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label May 1, 2023
@absidue absidue deleted the youtubejs-5.0.1 branch May 1, 2023 13:22
@Gorrrg
Copy link

Gorrrg commented May 1, 2023

$ node _scripts/build.js • electron-builder version=23.6.0 os=10.0.20348 • packaging platform=win32 arch=x64 electron=22.3.2 appOutDir=build\win-unpacked ⨯ cannot resolve https://github.com/electron/electron/releases/download/v22.3.2/electron-v22.3.2-win32-x64.zip: status code 503

Build script had trouble downloading a package.

@absidue
Copy link
Member Author

absidue commented May 1, 2023

You don't have to build it yourself anymore, as it's been merged you can just download the nightly build for it.

@Gorrrg
Copy link

Gorrrg commented May 1, 2023

Oh sorry, forgot to mention. That was from the nightly build log. Your win x86 builds failed. But the next nightly 2890 succeeded, so it's good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Network download and buffering is significantly slower compared to browser
6 participants