Skip to content

fix(Playlist): Only try extracting the subtitle for the first page#465

Merged
LuanRT merged 1 commit intoLuanRT:mainfrom
absidue:fix-playlist-subtitle
Aug 6, 2023
Merged

fix(Playlist): Only try extracting the subtitle for the first page#465
LuanRT merged 1 commit intoLuanRT:mainfrom
absidue:fix-playlist-subtitle

Conversation

@absidue
Copy link
Collaborator

@absidue absidue commented Aug 6, 2023

Playlist continuations don't have the header so we should only try extracting the subtitle for the first playlist page.

closes #464
bug introduced in #458, which added support for the subtitle

...this.page.metadata?.item().as(PlaylistMetadata),
...{
subtitle: header.subtitle,
subtitle: header ? header.subtitle : null,
Copy link
Contributor

@ChunkyProgrammer ChunkyProgrammer Aug 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rest of this file uses header?. instead so i'm not sure if header?.subtitle would be preferred

Ex:
can_share: header?.can_share,
can_delete: header?.can_delete,

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using header?.subtitle would result in it returning undefined, which would change the type of the subtitle property from Text | null to Text | null | undefined. This could be mitigated by doing header?.subtitle ?? null, but due to YouTube.js targetting ES2016, it gets transpiled to the rather verbose: (_b = header === null || header === void 0 ? void 0 : header.subtitle) !== null && _b !== void 0 ? _b : null,.

I can change the type of subtitle to be Text | undefined, if you would prefer that over null, I definitely don't want both null and undefined being possible?

@LuanRT LuanRT merged commit e370116 into LuanRT:main Aug 6, 2023
@absidue absidue deleted the fix-playlist-subtitle branch August 7, 2023 06:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5.8.0 header.subtitle failed due to header being undefined in Playlist constructor

3 participants