-
Notifications
You must be signed in to change notification settings - Fork 29
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
don't send entire library on each evaluation #1
Comments
Have you had any more thoughts on this performance enhancement? It seems rough that we send 150kb of JS on each lookup. |
Seemed rough to me too, yes, but empirical evidence from my usage of this library so far hasn't really run into this as a bottleneck yet. Are you running into this as being particularly slow for your tests? |
Im seeing a significant slow down for each query compared to using Given a small-ish page with a
Whereas 25ms is not super slow on its own, it is an order of magnitude slower than page.$ and nearly 40 times slower than evaluateHandle. I'd be curious how much of this could be sped up by somehow caching the evaluation on the browser side. It is of course possible that that the bulk of this time difference is just in the querying of the dom performed by dom-testing-library in which case we could not do much to fix it, but I'd like to determine that. |
getByText is not really a fair comparison to page.$(nodeType). I would be astonished if we were ever able to get it within an order of magnitude for any real world example when one requires examining all text nodes while the other is a browser-accelerated querySelector. That aside, it's certainly possible to enable the single eval of the library and maybe cut that time in half or more. Its just more complicated than the current setup. Feel free to take a stab if you'd like. |
Also note that the first invocation wouldn't improve at all with this change, only subsequent invocations. |
right now the entire dom-testing-library is sent on each evaluateAsync call to avoid execution context change problems (and global pollution), instead it could be installed just once on each page and reused through some sort of
.install()
mechanism or simple cached lookupThe text was updated successfully, but these errors were encountered: