Code Design Recommendations

Hello, I am the lead developer of the Rehike project, and I have a fair amount of experience working with ECMAScript and I know how to design good code. If I take it correctly, you seem to have some beginner level code that has some simple mistakes that complicate the flow of your code a lot.

For example, look at this section of the main JS file that forms a triangle with nested if statements. There are a few bad things going on, notably, a lack of proper formatting and an unclear use of asynchronous code. I recommend that you format long conditional operations like this to use && rather than using multiple nested if statements. That is to say:

// Instead of this:
if (flags.hasOperationA) {
    if (flags.hasOperationB) {
        if (flags.hasOperationC) {
            operationD();
        }
    }
}

// Do this:
if (
    flags.hasOperationA &&
    flags.hasOperationB &&
    flags.hasOperationC
) {
    operationD();
}

This approach not only makes it more clear to you and other developers what is going on here and doesn't require scrolling in the same way, but it also makes you intent more obvious and looks nicer to the eyes. It's just all around a better way to write the same thing.

I also reckon you're unfamiliar with asynchronous ECMAScript, which is fine because I made many mistakes when I first started working with it too. I highly recommend you build your APIs around Promises rather than pure setInterval or setTimeout calls. It sounds complicated at first, especially if you're still thinking with a synchronous mindset like I did, but it is so much easier than what you're doing right now. A benefit of using Promises is you can break up your tasks system that you used in the aforementioned examples into multiple functions that each return a Promise, with a master function that returns a status based on a composite response of the Promises. To put that simply, you can easily implement a nuanced API that has error handling functionality and so on while just using regular function-based programming like you are using. It'll be less global state to keep track of, easier to understand and maintain, and more stable.

Finally, there are quite a few behavioral quirks of how Polymer YouTube works that I haven't really written documentation on, but they're worth knowing, especially for a project of your calibre, and I wish to inform you of them. I'll share some code snippets and techniques I've used in my various Polymer modifications I've made which you can also use for certain purposes involving the modification of any Polymer HTML.

Polymer or how YouTube implements it (I'm not sure which) is very odd in that it reuses elements (and in a very weird way too). I am not sure how any of it works or the motivations for implementing this in such a manner. All I can tell you is that once a custom element (ytd elements) is created, it will not be deleted by YouTube's scripts, but simply moved to outside the DOM. Yes, you can actually do this. I believe that you can use DOMDocument objects in ECMAScript or something similar to create a user-inaccessible element, even through Inspect Element. Though, obviously, I haven't looked into this much to prove that this is something that they do.

You can get the InnerTube API data of most ytd elements by accessing their data property. As an example, document.querySelector("ytd-app").data will give you data for the entire website, including the current page and some miscellaneous information as well. This is provided in a pretty consistent format. For the main watch page contents, the data schema has hardly changed since 2016, which marks our earliest sight of it, so it has pretty much been the same for as long as we know. You can also modify this, to some degree, on certain elements, although that gets pretty complicated quickly.

If you are modifying the data of an element, it can often help to "data cycle" it. This is a term we have adopted in my little community for forcing an element to refresh its data. Here are the two methods that we developed to do this:

// The simple, "classical" method. Simply refreshes the same element:
var dataCopy = elm.data;
elm.data = {};
elm.data = dataCopy;

// The new, advanced method. Creates a new copy element to replace the original:
var newElm = elm.cloneNode();
newElm.data = elm.data;
for (var i in elm.properties)
    newElm[i] = elm[i];
elm.insertAdjacentElement("afterend", newElm);
elm.remove();

I use this code snippet a lot to wait for elements that I am certain that are going to exist:

async function waitForElement(elm)
{
    while (null == document.querySelector(elm))
    {
        await new Promise(r => requestAnimationFrame(r);
    }
    return document.querySelector(elm);
}

Finally, you can look through my profile or ask me further questions wherever about this. I have a good repo where I demonstrate use of a lot of what I described here called yt-anti-shorts. I highly recommend that you look through it no matter what you intend on doing with this project, since I think it is a very high quality project for modifying Polymer reliably.

Furthermore, you can listen for the yt-page-data-updated event, which fires when page navigation is complete and the page data and HTML has been updated.

Okay wow. I just wanna say thanks for this! While I don't understand most of this yet (I'll need to look over it again at least a few times), It looks like it could be very helpful for me.

Straightaway, I'll look into fixing that "triangle" problem probably for the next update, since it doesn't look too difficult to fix. Also yes, your method looks WAY better and easier to manage.

If I take it correctly, you seem to have some beginner level code that has some simple mistakes that complicate the flow of your code a lot.

You got that right. I'm taking a "learn through experience" approach with JavaScript, just like I did with CSS. It's helping so far, but I clearly still have a long way to go.

I also reckon you're unfamiliar with asynchronous ECMAScript

I don't even know what ECMAScript is... (Unless It's another word for JS which I'm assuming it isn't)

I highly recommend you build your APIs around Promises rather than pure setInterval or setTimeout calls. It sounds complicated at first, especially if you're still thinking with a synchronous mindset like I did, but it is so much easier than what you're doing right now.

I honestly don't understand what Promises are yet, though I haven't actually tried to figure that out yet. As far as I know, I'm learning this coding stuff quite quickly, considering I'm learning pretty much entirely on my own through experience, so here's hoping I'll be able to pick up on Promises quickly as well. Especially if it'll make my life easier.

All I can tell you is that once a custom element (ytd elements) is created, it will not be deleted by YouTube's scripts, but simply moved to outside the DOM.

Oh? Just to be clear, you're talking about the elements that you straight up can't find in devtools once they're "gone", and not how it adds the [hidden] attribute to invisible elements, right? In that case that is VERY weird.

You can get the InnerTube API data of most ytd elements by accessing their data property. As an example, document.querySelector("ytd-app").data will give you data for the entire website, including the current page and some miscellaneous information as well.

Okay, that sounds like it COULD be very helpful. However, I'm probably more of an amateur than you think. I have no idea what I'm supposed to do once I've grabbed the "data". Doing a console.log on the data from ytd-app returns undefined. I'm guessing "data" is not in a format that's meant to be put in a console.log.

I have a good repo where I demonstrate use of a lot of what I described here called yt-anti-shorts. I highly recommend that you look through it no matter what you intend on doing with this project, since I think it is a very high quality project for modifying Polymer reliably.

I've very briefly looked through the anti shorts script, and I'm already completely lost by line 50. Obviously I need to give it a closer look, but I think I'll probably need to learn about Promises first.

Anyway, I still need to look through this at least a few more times like I said. If I come to understand it better, I'll probably comment again. Once again thank you for taking the time to give me this information.

I don't even know what ECMAScript is... (Unless It's another word for JS which I'm assuming it isn't)

It actually is. ECMAScript is the official name of the language as it is standardised in ECMA-262. I guess I am overly pedantic by using it, but I prefer using the name for clarity's sake.

I honestly don't understand what Promises are yet, though I haven't actually tried to figure that out yet. As far as I know, I'm learning this coding stuff quite quickly, considering I'm learning pretty much entirely on my own through experience, so here's hoping I'll be able to pick up on Promises quickly as well. Especially if it'll make my life easier.

Promises are a standard API intended to achieve basically what you're already doing. That means programming in an asynchronous style, where functions can occur at a non-specific order. If you understand how events work, where you pass in a function name (this is called a function pointer) to an event listener that is then called by an external source (usually the browser itself) at a later point in time, then you already have a basis for understanding asynchronous code. Promises are just a really simple way to work with these in a nice way.

Right now, JS has two separate ways to handle Promises in code, which I will dub the classical and simple methods. Despite these names, both have their own places to be used for code clarity, so one is not fully preferable over the other. They are just different ways to write the same code.

Here is how the classical way works:

// You have a function that returns a Promise:
function sayHelloAfterASecond()
{
    // This is a top-level function return. The `new Promise` is returned instantly, so you
    // don't have to do anything. That second function is an anonymous function, which
    // is one that you think of as created right when it's needed. It is executed instantly,
    // too, but it doesn't "resolve" and tell what is attached to it until its resolve() function 
    // is called.
    return new Promise(function(resolve, reject) {
        setTimeout(function() {
            // resolve() carries over to further children; you can pass it around like any object.
            resolve("Hello world");
        }, 1000);
    });
}

// Because this is an asynchronous function, and the response is asynchronously returned,
// it cannot simply be accessed in a variable like this. This variable will instead be a Promise
// object that has handler functions that can be called on it.
var hello = sayHelloAfterASecond();

// Because it is a Promise, this will fail if you try it:
// "[object Promise]" instead of "Hello world"
console.log(hello);

// However, you can await its "promised" response by attaching a then callback using a new
// anonymous function. This will be called once the Promise body calls resolve().
hello.then(function(result) {
    console.log(result); // "Hello world"
});

So that's a lot to unpack probably, but it will make a lot of sense once you familiarise yourself with this design a lot more. I'm sure you already understand anonymous functions, which in JS are basically the same as regular functions. In fact, you can create a regular function for use for these purposes, which can be useful for code reuse. In that case, you will do it just like you do your setTimeout handlers and whatever in your regular code.

Promises are objects that wrap future values, so you can't handle them as normal variables, however I will get into how they managed to fake them acting like this in the "simple" design in a second.

As for a little more to say about Promises, they are a form of what is known as a messaging system. In the case of Promises, there can only be one recipient at a time, which is contrasted with other forms of messaging systems like publish/subscribe systems, which can have multiple recipients of the same data across your codebase. This design means that you have a messenger (the Promise itself) which can send out a "message" to a recipient, which will receive this message. The message can be any variable. When you call .then() on a Promise, you are telling it that this next function is where it should send its data once the Promise is resolved.

I didn't go over calling reject() in the Promise body or calling .catch() on the Promise itself, but it's basically the same thing, just a different road for handling errors specifically.

Promises are already nice, but they are a complete API, meaning once you separate your code into multiple distinct Promises that can form a whole, you can use things like Promise.all(promiseA, promiseB, promiseC, ...).then(...) to wait for multiple Promises and get the results of all of them at once.

Next up! The "simple" method:

Since ECMAScript 7, you can use async function (or function async, it works either way lol) to declare a function that uses Promises under the hood, but otherwise looks like a normal function. These functions can use the await keyword, which is like an automated .then() on a Promise that moves everything in the async function after that into that invisible .then() basically. These functions will always return a Promise. return in this type of function resolves the Promise, and throw will reject it.

And since async functions are just a syntax wrapper for Promises, you can use all Promise-based APIs with them (and vice versa).

But these are a bit limited in that you cannot resolve them within a child function, so you will either need to create Promise-based APIs for other language constructs (like setTimeout) or you will need to use regular Promise functions for portions or the entirety of these.

Here is the above example translated into an async function style:

// Since setTimeout is not a Promise-based API, you will need to create one for it.
// This is very easy, you just create a Promise that resolves when it does:
function timeout(durationMs)
{
    // This is an alternate anonymous function style you can use for purposes like this.
    // It works almost exactly the same (not entirely :P). These are called arrow functions.
    return new Promise( (resolve, reject) => {
        setTimeout(function() {
            resolve();
        }, durationMs);
    } );
}

// This function will return a Promise that can be awaited or `.then()`'d to get the string
// "Hello world", just like a regular Promise.
async function sayHelloAfterASecond()
{
    await timeout(1000);
    return "Hello world";
}

// You can't use "await" outside of an async function, so here's another one that doesn't
// return anything (a void function) that will act as a top-level for this.
async function main()
{
    console.log(await sayHelloAfterASecond()); // "Hello world"
}

// Alternatively, you can use the `.then()` style just the same as before.
sayHelloAfterASecond().then(result => console.log(result)); // "Hello world"

Enough with Promises! That probably got repetitive fast. But I am back to talking about YouTube now lol.

Oh? Just to be clear, you're talking about the elements that you straight up can't find in devtools once they're "gone", and not how it adds the [hidden] attribute to invisible elements, right? In that case that is VERY weird.

Indeed. This is almost something you can't notice at all because it just looks like they delete the elements, so it is very weird how they go about this. I am not sure if this is supposed to provide any performance benefit, but it will almost always result in constantly growing memory usage without it ever going back down. It's very strange that they would do things this way to me.

I'm not sure if you ever encountered this bug before, but for a very, very long time (and it probably still exists), there was a bug on the channel page where the video player for the channel trailer would continue playing after being moved out of the DOM completely. You could literally delete everything in inspect element and the audio would continue playing. It would be really nice if the browser let you access virtual DOMs created in JS like through the DOMDocument API in there. At least, I believe that is how they managed to pull this off.

Okay, that sounds like it COULD be very helpful. However, I'm probably more of an amateur than you think. I have no idea what I'm supposed to do once I've grabbed the "data". Doing a console.log on the data from ytd-app returns undefined. I'm guessing "data" is not in a format that's meant to be put in a console.log.

You're doing the call too early, before the page finishes loading and YouTube's JS is done setting up. If you run it in the console in inspect element during regular execution, it should be alright. I assume you got this error from trying to call it from within the extension, where it would probably run far too early. Thankfully, YouTube provides an event for listening to when the page changes (probably for their own internal use, but we can take advantage of it anyways). document.addEventListener("yt-page-data-updated", cbFunction) will subscribe to this event and call the callback function every time the page is changed (including the first load). I should add that this will occur before the page is visually updated, so the HTML could be unmodified if you listen for this, but this also means that it can be significantly easier to read the data from it.

This data can be console logged, as it is just an object. It will output a tree that you can navigate.

alright im gonna go take a shower now peace LOL

damn this shit derailed, better to close this issue & continue arguing in discord dms.

Okay, getting back on track, I decided to try both methods of Promises you mentioned, and I was able to get them to work using my Learn Through Experience™ method. I made sure to change them up at least somewhat, so I actually learn. The purpose is to determine whether the site is using Light or Dark mode, but I'm sure you could figure that out. Here are the examples, which both function as intended:

The classical method, which only took a few minutes to figure out:

function getSiteTheme()
{
    return new Promise(function(resolve, reject) {
        setTimeout(function() {
            if (document.querySelector("html[dark]") != null) {
                resolve("Dark theme");
            } else {
                resolve("Light theme");
            }
        }, 1000);
    });
}
var getTheme = getSiteTheme();
getTheme.then(function(theme) {
    console.log(theme);
    if (theme == "Light theme") {
        console.log("now doing stuff for when light theme is detected");
    }
    if (theme == "Dark theme") {
        console.log("now doing stuff for when dark theme is detected");
    }
    if (
    theme != "Dark theme" &&
    theme != "Light theme"
    ) {
        console.log("This is literally impossible and will never happen.");
    }
});

And the simple method (which actually took much longer for me to figure out):

function timeout(durationMs)
{
    return new Promise( (resolve, reject) => {
        setTimeout(function() {
            resolve();
        }, durationMs);
    } );
}
async function getSiteTheme()
{
    await timeout(2000);
    if (document.querySelector("html[dark]") != null) {
        return "Dark theme";
    } else {
        return "Light theme";
    }
}
getSiteTheme().then(theme => doNextFunction(theme));
async function doNextFunction(theme) {
    if (theme == "Light theme") { 
        console.log("now doing stuff for when light theme is detected");
    }
    if (theme == "Dark theme") {
        console.log("now doing stuff for when dark theme is detected");
    }
    if (
    theme != "Dark theme" &&
    theme != "Light theme"
    ) {
        console.log("This is literally impossible and will never happen.");
    }
}

Neat! I think I do understand what's going on here for the most part, at least with code that I just sent here. Hopefully I'm on the right track! While I can't think of many situations to use this in right now, I feel like this is one of those things where I will be hit with a wave of them in time!

I also don't fully understand the more complicated details, but I feel that will come with time. Once again I'll probably be looking over your explanation of it again later on.

Indeed. This is almost something you can't notice at all because it just looks like they delete the elements, so it is very weird how they go about this. I am not sure if this is supposed to provide any performance benefit, but it will almost always result in constantly growing memory usage without it ever going back down. It's very strange that they would do things this way to me.

Yeah, I just tried to think of a valid reason for why they would do that, but came up empty.

I'm not sure if you ever encountered this bug before, but for a very, very long time (and it probably still exists), there was a bug on the channel page where the video player for the channel trailer would continue playing after being moved out of the DOM completely.

Actually I think I might've encountered that like 2 days ago, but I could be wrong, I wasn't really thinking about it. I never checked the devtools though, I just refreshed. I do know that something kept playing on the channel page though.

I assume you got this error from trying to call it from within the extension, where it would probably run far too early.

Nope, I don't think doing a setTimeout for 12.5 seconds is too early. I must be doing it fundamentally wrong or something.

// Quick and dirty function for getting ytd-app's data that doesn't work

setTimeout(tempFunct, 12500);

function tempFunct() {
    var ytdData = document.querySelector("ytd-app").data;
    console.log("got data");
    setTimeout(tempFunct2, 1000);
    function tempFunct2() {
        console.log(ytdData); //undefined
    }
}

If you run it in the console in inspect element during regular execution, it should be alright.

Yeah, I was able to get it to work in devtools now after trying it.

Thankfully, YouTube provides an event for listening to when the page changes (probably for their own internal use, but we can take advantage of it anyways). document.addEventListener("yt-page-data-updated", cbFunction) will subscribe to this event and call the callback function every time the page is changed (including the first load).

So basically, I should try doing it with this event thing instead? I haven't tried yet, admittedly.

Yep, you got the classical Promises method right. Not the simple method though. I admit, it is actually a lot harder to explain compared to the simple method. await binds to whatever comes after it and waits until that Promise is resolved, so it's a dedicated language keyword to working around Promises basically. It can make it easier to convert regular functions into asynchronous ones, especially if they don't return anything at all.

Nope, I don't think doing a setTimeout for 12.5 seconds is too early. I must be doing it fundamentally wrong or something.

Actually I'm not sure why it's behaving this way. It might indeed be too early or some weird JS behaviour I don't understand. I recommend trying to log it using the event method instead. Come to think of it, I think my friend had a similar issue with accessing certain page data through a WebExtensions extension way back in the day, whereas I have experience exclusively with userscripts. A few years ago, we wrote a complex script to use the DOM to message this information, but I am not sure if it was of any importance or if we were overcompensating because we couldn't figure out a simpler way to do it at the time. If it doesn't work with the event, then I'm sure it's some really weird extensions issue that needs to be worked around.

Yep, you got the classical Promises method right. Not the simple method though.

So it's a situation where even though I got what I was looking for with the simple method, I wasn't using it in a way that actually takes advantage of Promises? Either way, I'm sure I'll understand eventually, and I'll look back on this code and cringe.

A few years ago, we wrote a complex script to use the DOM to message this information, but I am not sure if it was of any importance or if we were overcompensating because we couldn't figure out a simpler way to do it at the time.

This script was to get it to work with WebExtensions? Do you still have this script?

If it doesn't work with the event, then I'm sure it's some really weird extensions issue that needs to be worked around.

Seems like that's probably it. I tried the same code in a userscript, and it worked just fine. After at least an hour of trying, I got this to work in the extension:

createCollector();
function createCollector() {
    let container = document.querySelector('html');
    const newElem = document.createElement("div");
    newElem.id = 'bt-collector';
    newElem.setAttribute("class", "bt-universalized-element");
    newElem.innerHTML = `
    <input id="get-data-attribute" autofocus onfocus='
    document.addEventListener("yt-page-data-updated", cbFunction); 
    function cbFunction() { 
        var ytdData = document.querySelector("ytd-app").data; 
        console.log(ytdData); 
    } 
    document.querySelector("#get-data-attribute").blur();
    '></input>
    `;
    container.insertBefore(newElem, container.children[0]);
}

Not sure if it'll even be useful like this though. Unless I can somehow get it so outside functions can access ytdData, but I don't know how to do that/if it's even possible. But any attempt to do it "out in the open" (if that makes any sense) resulted in undefined, including the yt-page-data-updated method.

You might be able to create a script that executes inside the document to work around this. Otherwise I'll have to ask my friend how he managed to work around it. I think worst of is all this is a Firefox only issue with extensions, where you would be able to access it normally from an extension in Chrome.

For example:

var a = document.createElement("script");
a.textContent =
`console.log(document.querySelector("ytd-app"));`
document.body.insertAdjacentElement("beforeend", a);

So it's a situation where even though I got what I was looking for with the simple method, I wasn't using it in a way that actually takes advantage of Promises? Either way, I'm sure I'll understand eventually, and I'll look back on this code and cringe.

Promises can wrap anything that is asynchronous, so you can have a Promise resolve on the end of a setTimeout or setInterval, or at the first (or second, or third, etc.) time of an event being fired, or observers, or anything else like that.

If you do network requests, the Fetch API already uses Promises. Otherwise you will can also use Promises to wrap XMLHttpRequest API.

You might be able to create a script that executes inside the document to work around this.

Yes, that was part of it. I found a workaround which makes use of localStorage:

var a = document.createElement("script");
a.textContent = `
document.addEventListener("yt-page-data-updated", cbFunction); 
function cbFunction() { 
    var getYtdData = document.querySelector("ytd-app").data; 
    storeData();
    function storeData() {
        localStorage.setItem("ytd-app-data", JSON.stringify(getYtdData));
    }
    // Modifications to the data must be done here
} 
`
document.body.insertAdjacentElement("beforeend", a);

document.addEventListener("yt-page-data-updated", retrieveDataAttribute); 
function retrieveDataAttribute() {
    var ytdData = localStorage.getItem("ytd-app-data");
    console.log(JSON.parse(ytdData));
    var convertedYtdData = JSON.parse(ytdData);
    if (convertedYtdData.page == "watch") {
        console.log("Watch Page Detected");
    }
    if (convertedYtdData.page == "browse") {
        console.log("Browse Page Detected");
    }
    if (convertedYtdData.page == "channel") {
        console.log("Channel Page Detected");
    }       
    if (convertedYtdData.page == "search") {
        console.log("Search Page Detected");
    }   
}

Yep, this grabs the new data every page load! It's not 100% ideal, as the data can't be modified from outside the newly created script, but I guess I'll just have to deal with that. Besides, I probably won't be modifying the data for a while yet. But for now I think I might experiment with having stuff like the view count be grabbed from the data, instead of being taken from YouTube's view count element itself. It seems like it might be more reliable.

Anyway thanks for the suggestions! I've learned alot in the past few days. I've already decided that I'm going to spend more time on the next update than I originally was. While I'm sure the next update will still have issues, hopefully it will be a noticeable improvement. I'll let you know if I have any more questions.

Edit: I ended up changing it to sessionStorage, because localStorage seems to affect all tabs, instead of the active one. Same idea though.

Okay, so I just released version 1.0.4. It's a smaller update than I originally intended since I needed a short break from working on this extension, so it doesn't include too many code improvements. But it does fix the triangle problem, and I've started to make use of the data property, as well as yt-page-data-updated. However I have not started on moving to asynchronous code yet. There's much more to be done still, and I hope to address more of these issues in future updates.

lightbeam24 / CustomTube

Code Design Recommendations #10