fix: injectJQuery in context does not survive navs

[barjin]

Let's play Bug Fix or a Breaking Change!

Because requestHandler and pre/postNavigationHooks share the same context, the surviveNavigations is disabled for injectJQuery in all of those handlers/hooks.

Consider the following snippet:

{
 preNavigationHooks: [
   async ({ injectJQuery }) => injectJQuery();
 ],
 requestHandler: async ({ page })  => {
   await page.evaluate(() => $('title').text);
 },
}

This could have worked before (albeit it was very unstable), but will fail every time now.

What is it then? Bug Fix or a Breaking Change?

[B4nan]

i think we can do better, detecting whether the execution happens from inside the request handler should be easy as it all goes through BasicCrawler._runRequestHandler, similarly, we can detect the preNavigationHooks though BrowserCrawler._handleNavigation.

just to be sure, it won't work with pre nav hooks, but it will keep working with post nav hooks (as well as with direct request handler usage)

[barjin]

Okay, I understand we want to alter the behavior (turn off surviveNavigation) in requestHandler and postNavHooks as well - maybe output a warning message in preNavigationHooks?

Either way, this should be done in playwrightUtils module (sounds not SOLID to handle this anywhere else), which leaves us with analyzing the call stack there? Altering the behavior based on the stack contents sounds like a massive antipattern - but I'm probably missing something. What solution do you have in mind?

[B4nan]

Stack traces are rather expensive and error-prone, I don't want such a solution to be done on a per request base.

What I was thinkin about is a simple bool flag on the request object maybe, that would tell you in what processing state the request is - pre/post nav hooks, or request handler (or maybe something else too, idk), maybe an enum instead of multiple bools, didn't put much thinking to it. You'd use that in here where you have the full context object available:

crawlee/packages/playwright-crawler/src/internals/utils/playwright-utils.ts

Line 728 in ffc53e7

context.injectJQuery = () => injectJQuery(context.page);

[B4nan]

lgtm, i see it was a bit more work than expected, but i like that we now have this information available, i am sure there will be more use cases where this will be very handy