Skip to content
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

[11.x] Preload base options for missing config files #51619

Merged
merged 2 commits into from
May 30, 2024

Conversation

jasonmccreary
Copy link
Contributor

@jasonmccreary jasonmccreary commented May 29, 2024

Laravel 11 allows you to remove configuration files to keep your application slim and easier to maintain. However, Dries recently brought to my attention some issues which may arise when configuration files are removed.

After digging into the code, the issue occurs when a config file references another value from a missing (removed) config file. From one of the issues, Sanctum referencing app.url when config/app.php has been removed.

This is because the application configuration files are loaded and merged with the base options, then any remaining base options are loaded.

This PR solves this issue by first loading the base options from any missing core configuration file first (i.e. app, broadcasting, view, etc). It is safe to preload these options as they do not exist and therefore do not need to be merged with user customizations.

There is no additional complexity as it loads the same amount of config files. It simply loops over any missing config files first to avoid invalid references. Furthermore, config may be cached, so this additional loop only runs once.

I did attempt to add a test for this, but it was pretty hacky as I needed to remove config files from the TestBench skeleton app for proper setup. Since that skeleton would also need to be maintained, it's not really worth it. If Orchestra ever adds a hook for swapping config files, I'll gladly backfill a test.

@driesvints driesvints changed the title Preload base options for missing config files [11.x] Preload base options for missing config files May 30, 2024
@taylorotwell taylorotwell merged commit 448fdb5 into laravel:11.x May 30, 2024
28 checks passed
@jasonmccreary jasonmccreary deleted the preload-missing-config branch May 30, 2024 14:23
@crynobone
Copy link
Member

crynobone commented Jun 7, 2024

@jasonmccreary You can always override the following method:

<?php

use Illuminate\Foundation\Bootstrap\LoadConfiguration;
use Orchestra\Testbench\TestCase;

class LoadConfigurationTest extends TestCase
{
    #[\Override]
    protected function resolveApplicationConfiguration($app)
    {
        $app->instance(LoadConfiguration::class, new LoadConfiguration());

        $app->useConfigPath(__DIR__.'/stubs/config'); // set new configuration path to avoid using Laravel skeleton from Testbench

        parent::resolveApplicationConfiguration($app);
    }
}

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants