Dev Suggestions

[fire332]

EDITED March 29

I've accumulated a list of minor suggestions/issues about the repo that I haven't gotten around to or don't feel appropriate to make a PR without some discussion.

• The build script needs to have --mode=production appended so that the bundle will actually be optimized during the GH release workflow. Consequently, there probably should be a build:dev script for convenience.
  • Fixed by Build in production mode by default #138
• Since top-level await has been supported by webpack by default since 5.83.0, this IIFE wrapper can be removed but testing is needed.
• Instead of using timers to determine when to skip in the sponsorblock code, we can check a list of ranges in a <video> timeupdate event listener. Using timers seems error prone and hard to update from a programming standpoint if we want to add features to the sponsorblock code.
• I've noticed the .vscode folder is now gitignore'ed. Is it intentional that the .vscode folder is not deleted from the repo? I've originally made a PR adding that folder for developer convenience but I don't know about @throwaway96's thoughts on that.
  • Addressed below.
• It's probably possible to make a script to launch chrome with the correct user agent and the userscript loaded as an extension via Playwright. This will make local testing easier. If there's interest, I can make a PR about this.
• If there's interest, I can convert the code to TypeScript in a PR.
• The domrect and spatial-navigation polyfills can be converted to patched dependencies to clarify the changes performed. Yarn and pnpm support patching dependencies natively, and patch-package can be used for npm.
• @throwaway96 to reply to your comment here, waitForChildAdd could take a required arg to determine whether or not to observe attribute mutations which will maintain the principle of least surprise. The signature change will be as follows:

// FROM
declare async function waitForChildAdd(parent: Element, predicate: () => boolean, abortSignal?: AbortSignal);

// TO
declare async function waitForChildAdd(parent: Element, predicate: () => boolean, observeAttributes: boolean abortSignal?: AbortSignal);

• Writing E2E tests looks viable with the use of WebDriverIO + Appium + a webOS-specific driver. Some work has been done on that front in headspinio/appium-lg-webos-driver

[throwaway96]

I don't have time to respond to everything right now, but:

• The build script needs to have --mode=production appended so that the bundle will actually be optimized during the GH release workflow. Consequently, there probably should be a build:dev script for convenience.

Agreed. #138

• I've noticed the .vscode folder is now gitignore'ed. Is it intentional that the .vscode folder is not deleted from the repo? I've originally made a PR adding that folder for developer convenience but I don't know about @throwaway96's thoughts on that.

I'm fine with having it contain default settings when initially cloning the repo. But nobody wants to commit their own .vscode dir, and it's easier to have it in .gitignore than for everyone to individually add it to .git/info/exclude.

• It's probably possible to make a script to launch chrome with the correct user agent and the userscript loaded as an extension via Playwright. This will make local testing easier. If there's interest, I can make a PR about this.

Good idea. We just have to hope YouTube isn't relying on anything WAM-specific. (I would be kind of surprised if they are, at least in a major way.)

• @throwaway96 to reply to your comment here, waitForChildAdd could take a required arg to determine whether or not to observe attribute mutations which will maintain the principle of least surprise. The signature change will be as follows:

// FROM
declare async function waitForChildAdd(parent: Element, predicate: () => boolean, abortSignal?: AbortSignal);

// TO
declare async function waitForChildAdd(parent: Element, predicate: () => boolean, observeAttributes: boolean abortSignal?: AbortSignal);

My preference is throwaway96/youtube-webos@b837753, but I suppose this is fine as a compromise. (See #145.)

[fire332]

eg2.vscode-npm-script needs to be removed from the extension recommendations as it has been deprecated.

See: https://github.com/Microsoft/vscode-npm-scripts?tab=readme-ov-file#node-npm

Resolved in #146