TakayoshiKochi / deprecate-style-in-html-imports

Deprecating/Removing <style> application from HTML Imports
https://takayoshikochi.github.io/deprecate-style-in-html-imports/
47 stars 44 forks source link

infinite loop in findStylesheetsInImportedDocs()? #11

Open jonsmithers opened 6 years ago

jonsmithers commented 6 years ago

I can't say I understand why this happens, but I was finding that the provided findStylesheetsInImportedDocs() would somehow get stuck in an infinite loop, revisiting the same imports over and over again. I think I avoided this appropriately with the following modification:

function findStylesheetsInImportedDocs(doc, map, visited) {
    map = map || {};
    visited = visited || new Set();
    Array.prototype.forEach.call(doc.querySelectorAll('link[rel=import]'), (e) => {
        const importedDoc = e.import;
        if (importedDoc && !map[importedDoc]) {
            const styles = importedDoc.querySelectorAll('style,link[rel=stylesheet]');
            if (styles.length) {
                map[importedDoc.URL] = {
                    doc: importedDoc,
                    styles: styles
                };
            }
            if (!visited.has(importedDoc.URL)) {
                visited.add(importedDoc.URL);
                findStylesheetsInImportedDocs(importedDoc, map, visited)
            }
        }
    });
    return map;
}
console.log(findStylesheetsInImportedDocs(document));
TakayoshiKochi commented 6 years ago

Do you want me to debug your code ? :=)

jonsmithers commented 6 years ago

I'm kind of just pointing out that the findStylesheetsInImportedDocs() method in the readme seems susceptible to infinite loops. After debugging and looking through the call stack, I discovered we have 4 elements that create a circular dependency, and findStylesheetsInImportedDocs() basically recurses on those 4 elements until you get this:

Uncaught RangeError: Maximum call stack size exceeded
    at Array.forEach.call (<anonymous>:6:32)
    at NodeList.forEach (<anonymous>)
    at findStylesheetsInImportedDocs (<anonymous>:4:29)
    at Array.forEach.call (<anonymous>:15:13)
    at NodeList.forEach (<anonymous>)
    at findStylesheetsInImportedDocs (<anonymous>:4:29)
    at Array.forEach.call (<anonymous>:15:13)
    at NodeList.forEach (<anonymous>)
    at findStylesheetsInImportedDocs (<anonymous>:4:29)
    at Array.forEach.call (<anonymous>:15:13)

The findStylesheetsInImportedDocs() method doesn't need to recurse into html imports it has already encountered. This is how you can avoid infinite recursion.

TakayoshiKochi commented 6 years ago

Ah, sorry, the code is in the readme. (I don't remember the code was what I wrote, or someone gave me... but definitely not in a style that I would write)

As the original bug (deprecating the styling part only from HTML Imports) was closed, so this may not be worth fixing and just close the page entirely.