Improve tooling

[fire332]

Check the first commit to see the actual changes.

• Added vscode, prettier, and eslint configs.
• Fixed eslint errors in the code.
• Updated package-lock.json to new lock file version.
• Made the deploy script multi OS compatible.
• Improved production bundle size. 174 KiB userScript.js + 922 B index.js -> 137 KiB userScript.js + 10.7 KiB index.js.
• Added pre-commit prettify hook.
• Polyfill index.js.
• Switched the spatial-navigation-polyfill to an NPM dependency.

Closes #16

[fire332]

I just remembered, the domrect polyfill is also unnecessary if we check for and use ClientRect instead as appropriate. However, this needs testing to be sure. Obviously I can't test this as I own a LG C1 which uses Chromium 79.

[fire332]

Whoops. @babel/runtime-corejs3 shouldn't be a dev dependency. Fixing now.

[Informatic]

DomRect is used by spatial navigation polyfill. Additionally - spatnav polyfill was pulled into out repo specifically becaused it had a bunch of patches to make it work properly on older browsers. (I don't remember the specifics and am slightly AFK for the next 12+ hours, but all the changes had comments marked like here: https://github.com/webosbrew/youtube-webos/blob/main/src/spatial-navigation-polyfill.js#L673 )

…

On Fri, 7 Jan 2022, 18:35 fire332, ***@***.***> wrote: I just remembered, the domrect polyfill is also unnecessary if we check for and use ClientRect instead as appropriate. However, this needs testing to be sure. Obviously I can't test this as I own a LG C1 which uses Chromium 79. [image: image] <https://user-images.githubusercontent.com/96039230/148583665-e4e27f67-29bd-44fa-b473-555654e80d16.png> — Reply to this email directly, view it on GitHub <#19 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIUEXVXH7FSGOYDHYAWUQLUU4P45ANCNFSM5LPIIDIQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[fire332]

Ah ok, just saw your comment. You may want to make the changes more obvious as I couldn't tell from a quick glance. Reverting that portion now.

[fire332]

DomRect is used by spatial navigation polyfill.

I'm aware. What I'm saying is that, according to caniuse.com, DOMRect is available as ClientRect on Chrome version 4-60 so the DOMRect polyfill can be removed if ClientRect is tested to be working properly.

[Informatic]

Hey, sorry for the delay.

Can you please check if SponsorBlock segments are properly rendered on the timeline on Your TV? I can consistently make them work on current main (on this video), but can't on this branch. No obvious

Oneliner:

git stash && git checkout main && npm install && npm run build && npm run package && npm run deploy && npm run launch -- -p '{"contentTarget":"v=ptnXLYYIQ9Y"}'

I see an undefined is not a function error occuring on this line:

youtube-webos/src/sponsorblock.js

Line 171 in 06b8bc9

m.removedNodes.forEach((n) => {

Indeed - on main NodeList.prototype.forEach is a properly shimmed native function, while it doesn't seem to be shimmed in this branch.

[fire332]

Is it only that specific instance of NodeList missing the .forEach or is .forEach missing on the global NodeList's prototype?

[Informatic]

That's on global NodeList prototype (NodeList.prototype.forEach I mentioned)

[fire332]

Can you check to see if this works?

[Informatic]

Tested on 9637280, sadly, still doesn't work :/

NodeList.prototype.forEach is still undefined, and the same Uncaught TypeError: undefined is not a function still occurs.

[fire332]

Try again with polyfill-corejs3's method set to 'usage-global' instead of 'usage-pure'.

[Informatic]

Ah, indeed. Now with usage-global and core-js-pure dependency replaced with core-js (otherwise it'd fail on missing core-js with that previous option changed) it works correctly now it seems. I'll do deeper testing in a couple of hours, but looks great so far. Thank you.

[fire332]

Talked to the core-js dev. Seems like a Babel bug that NodeList.prototype.forEach is not pollyfilled in pure mode and looking at the Babel issue tracker, doesn't seem like this will be fixed any time soon. Better if we just migrate all usage of .forEach on NodeLists to use for...of syntax instead.

[fire332]

Please test the follow commit. I'll rebase them all if this is good.

[Informatic]

Works perfectly now. Thanks.

[Informatic]

LGTM

[fire332]

Please don't merge before I get around to rebasing, unless you truly don't mind having commits labeled test and test2 in the repository.

[Informatic]

Sure, though squashing would probably work just fine in our case. (GitHub is able to do that automatically right before merge)

…

On Fri, 14 Jan 2022, 00:14 fire332, ***@***.***> wrote: Please don't merge before I get around to rebasing, unless you truly don't mind having commits labeled test and test2 in the repository. — Reply to this email directly, view it on GitHub <#19 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIUEXSBEAJRA4EHD4OBPQTUV5MF5ANCNFSM5LPIIDIQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you commented.Message ID: ***@***.***>

[fire332]

I recommend having a separate commit just for the prettify command as it touches almost every file, making the actual changes a pain to diff. Rebasing done. Good to merge.

[Informatic]

Yeah that makes sense. I suspect you're going to need to rebase other PRs as well after we merge it in.