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

Ensure DOM is settled #225

Closed
wants to merge 4 commits into from

Conversation

gladyshcodes
Copy link
Contributor

Issue #217

Proposed change
Get rid of setTimeout brute force

Why
Ineffective (see why in the issue)

Outcome / numbers:
Process that took as a 1s to perform now takes about 1-5 ms (that's how much it actually takes for DOMContentLoaded event to be emitted)

However, since the pages Shortest navigates through may experience DOM mutations even after the initial page load (e.g., when modals are opened), we continue to wait until all DOM mutations are completed using MutationObserver

Copy link

vercel bot commented Dec 31, 2024

@gladyshcodes is attempting to deploy a commit to the Antiwork Team on Vercel.

A member of the Team first needs to authorize it.

@gladyshcodes
Copy link
Contributor Author

gladyshcodes commented Dec 31, 2024

@slavingia FYI Currently, implementing such functionality without Playwright (as we discussed in #217) will be challenging and time-consuming as Playwright has its own internal lifecycle events that it emits / consumes

When trying to do this natively I receive:

Failed to wait for stable DOM: page.evaluate: Error: Timed out after 1000ms waiting for the DOM to stabilize.
at eval (eval at evaluate (:234:30), <anonymous>:5:15)

See how Playwright listens to events
See how Playwright emits events

I think we could always start with Playwright and then deprecated it with .evaluate and other deps altogether

@slavingia
Copy link
Contributor

Sounds good, we can keep playwright for now!

Copy link

vercel bot commented Jan 2, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
shortest ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 2, 2025 10:07pm

@m2rads
Copy link
Contributor

m2rads commented Jan 3, 2025

Looks good! Can we add this method for Github and Mailosaur tool as well? @gladyshcodes

@gladyshcodes
Copy link
Contributor Author

I guess I will close this PR as many fundamental changes will be made to those files in upcoming PR for issue #227. FYI @slavingia @m2rads

Looks good! Can we add this method for Github and Mailosaur tool as well? @gladyshcodes

Okay, will make sure it's used everywhere across the codebase

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

Successfully merging this pull request may close these issues.

3 participants