Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] Conflicting line of code in src/dom.js causing infinte loop. (waitForDocumentBody) #33

Open
Cijin opened this issue Oct 28, 2020 · 2 comments

Comments

@Cijin
Copy link
Contributor

Cijin commented Oct 28, 2020

The function checks if the document body is present. If the document is not ready, it waits for the window to be ready and checks again if the document body is present. If the body is present, it returns the body. If not throws an error. This is the code:

 return waitForDocumentReady().then(() => {
            if (document.body) {
                return document.body;
            }

            throw new Error('Document ready but document.body not present');
        });

But, waitForDocumentReady calls isDocumentReady which in turn checks:

if (document.body && document.readyState === 'complete) return true`

That means the document cannot be ready without the body being ready. This causes an error while testing, when I mocked document.body = null. Also leads to throw being unreachable.

Cijin pushed a commit to Cijin/belter that referenced this issue Oct 28, 2020
@bluepnume
Copy link
Collaborator

bluepnume commented Oct 28, 2020

I agree the throw is unreachable. That may just be an artifact of making Flow happy (since document.body is always an optional in Flow, throwing is the only way to make Flow happy that we are returning a definite HTMLBodyElement type rather than a ?HTMLBodyElement).

That said; I don't think we can change the behavior of waitForDocumentReady or isDocumentReady at this point, to not depend on document.body being present. That can be changed in a new major version of belter -- but if we remove that now, that is likely to be breaking behavior.

@Cijin
Copy link
Contributor Author

Cijin commented Oct 29, 2020

Yes, and to avoid that I could remove the conditional statement. Because if waitForDocumentReady has resolved that means document.body has to be present. That could make flow happy.

I agree with you on the second part. Changing waitForDocumentReady or isDocumentReady would cause breaking changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants