roach-php
Make ExecuteJavascriptMiddleware handle requests
This PR aims to close #239. By letting the middleware handle a request instead of a response and setting the response on the request, we ensure the request is only sent once.
Besides the fact that handling requests instead of responses saves a request, it also make sense for this middleware to handle the request instead of the client, since the request is send with the middleware.
One could say that a separate client should be created for sending Javascript requests and one would have a point in my honest opinion, but it's quite difficult to create Php responses from Browsershot/Puppeteer. Therefore, the changes in this PR offer a middleground.
I see your point and the implementation looks fine to me. I still have to check if skipping the client could have any unintended consequences for users. I don't think it does since the Downloader still calls onResponseReceived for requests that had their response set by one of the middlewares.
Please rebase your branch on top of the current main branch. That should get rid of most of the pipeline errors. Don't worry about the commitlint action. I can change the commit message when merging.
Thanks for taking the time to look at this PR, @ksassnowski.
I agree with your point and I don't think it will pose any issues for users. The only case I can think of is users that rely on both the client and the middleware, but I can't see why someone would need this.
@ksassnowski The failed checks don't have anything to do with this PR, do they? Can you help me out?
Yeah the failing pipeline seems to be related to a PHPStan upgrade. Don't worry about that for now, I'll fix that on the main branch after this gets merged.
I'll still have to do a final test run before merging this but it looks mostly good to me. Sorry for the delay!
No worries and thanks for taking the time. If I can help in any way, let me know!