laravel / valet

A more enjoyable local development experience for Mac.
https://laravel.com/docs/valet
MIT License
2.49k stars 691 forks source link

Support static caching in Statamic #1440

Closed jasonvarga closed 9 months ago

jasonvarga commented 10 months ago

Closes #1418

This PR adds a new driver for Statamic v3+.

Starting in v3, Statamic is just a Laravel package so we didn't have a need for a dedicated driver - the Laravel driver works fine. However as pointed out in #1418, our "static caching" feature wouldn't function 100% as expected locally because it assumes some custom server config. This driver "fakes" it.

In addition to the driver itself, it needs to be loaded before the Laravel driver. Otherwise, the Laravel driver will be used since a Statamic site also qualifies as a Laravel site.

Lastly, @aerni mentioned in the issue that we should call this driver "Statamic" and the previous one "StatamicV2" - I agree but it would be a breaking change. Not a big deal.

drbyte commented 10 months ago

Lastly, @aerni mentioned in the issue that we should call this driver "Statamic" and the previous one "StatamicV2" - I agree but it would be a breaking change. Not a big deal.

I'd be okay with (and prefer) that change (rename current one to V2, and let this V3 be the unversioned name) ... since I can't think of a logical case where anyone would have that filename hardcoded anywhere.

jasonvarga commented 10 months ago

Can do.

jasonvarga commented 10 months ago

Didn't realize there were tests for that - nice. Fixing those.

jasonvarga commented 10 months ago

Okay, done now. I've added tests and adjusted them so they all have appropriate fixtures corresponding to the renamed drivers.

drbyte commented 10 months ago

FWIW, in my testing I get the same results with either of these strategies:

The one currently in this PR:

+        $drivers[] = 'Specific\StatamicValetDriver';
         $drivers[] = 'LaravelValetDriver';
         $drivers = array_unique(array_merge($drivers, $specificDrivers));

and the other idea suggested in the related issue:

-        $drivers[] = 'Specific\StatamicValetDriver';

-        $drivers[] = 'LaravelValetDriver';
         $drivers = array_unique(array_merge($drivers, $specificDrivers));
+        $drivers[] = 'LaravelValetDriver';

Each approach has its pros and cons:

In the end, I don't mind which approach we use here. I'm fine with the hardcoding of the Statamic driver, as committed already. Just pointing out the additional testing results.

jasonvarga commented 10 months ago

Thanks. I'll write a test somewhere for the first/current change. I'd rather not move the Laravel driver in case it changes something.

drbyte commented 10 months ago

Totally understood.

Will leave it to @mattstauffer to merge etc.

mattstauffer commented 9 months ago

Thanks @jasonvarga and @drbyte!