Send Entire Request, Including Query Params, To Prerender

[kellerjmrtn]

Problem

Prerender accepts query params and allows them to influence the prerendered page content, see https://docs.prerender.io/docs/ignore-config. Some applications require this behavior

Solution

Send the query params along in the prerendered request

This behavior was removed in the previous version of this repository here jeroennoten/Laravel-Prerender#9. It's not exactly clear to me how this solves an ELB http/https issue, as fullUrl() likewise differentiates between the two using $request->isSecure(), so I'm not sure how or if this would reintroduce that issue

The only other change this makes is that when requesting the page at /, the new logic no longer sends a trailing slash (something like https://service.prerender.io/http%3A%2F%2Flocalhost rather than https://service.prerender.io/http%3A%2F%2Flocalhost%2F). All other urls should remain the same, with the exception of query params

If either of these may be an issue, we could also append the query string to the prexisting logic with $request->getQueryString() instead of using fullUrl(), I just figured the latter would be simpler

[StanBarrows]

Hey @kellerjmrtn

Thanks for your PR.

Because that could be a breaking change for some users, I suggest another approach.

Within this release, we extend the current implementation via a config parameter. Extend config.php with full_url and make default false. This way, it does not break any existing installation.

Extend config.php

   'full_url' => env('PRERENDER_FULL_URL', false),

Implement Function

Then, we implement a function with the existing & extended code. Careful, I just copy-pasted & coded this within this Github-Issue as an example.

function generatePrerenderUrl($request) {
	
    if (config('prerender.full_url')) {
        return $request->fullUrl();
    }

    $protocol = $request->isSecure() ? 'https' : 'http';
    $host = $request->getHost();
    $path = $request->path() === '/' ? '' : ltrim($request->path(), '/');

    return "{$protocol}://{$host}/{$path}";

}

$url = $this->generatePrerenderUrl($request);

return $this->client->get($this->prerenderUri . '/' . urlencode($url), compact('headers'));

Also, thanks for the suggestions for checking for the query parameters.

However, I would like to implement and optimize this when doing the next bigger update for the project to keep all existing installations intact. Even though this should not break any existing installations and should cover, in theory, all cases, let's avoid any risks.

We can decide when doing an update to Laravel 12. If we either set the Full URL config parameter default to true or replace it with the enhanced check for query parameters. #41

function generatePrerenderUrl($request) {
    $protocol = $request->isSecure() ? 'https' : 'http';
    $host = $request->getHost();
    $path = $request->path() === '/' ? '' : ltrim($request->path(), '/');
    
    $fullUrl = "{$protocol}://{$host}/{$path}";

    if ($request->getQueryString()) {
        $fullUrl .= '?' . $request->getQueryString();
    }

    return $fullUrl;
}

I'm currently out of the office until the 12th of August. So you either update your existing PR, and I would review & merge it ASAP, or update the code when I return.

Best regards
Sebastian

ToDo's

• Update config.php
• Implement function
• Update readme.md with the new approach & config.php example
• Update Tests

[kellerjmrtn]

Agree with your suggestion, maintaining backwards compatibility is much preferred. I've implemented your solution including the new config value and another test. Thanks!

[StanBarrows]

Thanks a lot for this! Tagged v11.2

https://github.com/codebar-ag/laravel-prerender/releases/tag/v11.2