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

Fix regression introduced in #337 #368

Closed
wants to merge 1 commit into from
Closed

Fix regression introduced in #337 #368

wants to merge 1 commit into from

Conversation

d8vjork
Copy link

@d8vjork d8vjork commented Feb 14, 2022

We cannot upgrade Inertia from 4.5.0 to 5.x as this introduces a new issue on our software.

Issue

image

Scenario

This is a piece of what we've in our app/Providers/RouteServiceProvider.php

   /**
     * These namespaces is applied to your controller routes.
     *
     * In addition, it is set as the URL generator's root namespace.
     *
     * @var array
     */
    protected $namespaces = [
        'web' => 'App\Http\Controllers',
        'api' => 'App\Http\Controllers\Api',
    ];

    /**
     * Define your route model bindings, pattern filters, etc.
     *
     * @return void
     */
    public function boot()
    {
        $this->configureRateLimiting();
        $this->configureRoutePatters();

        $this->routes(function () {
            Route::middleware('web')
                ->namespace($this->namespaces['web'])
                ->group(base_path('routes/web.php'));

            Route::prefix('api')
                ->middleware('api')
                ->namespace($this->namespaces['api'])
                ->group(base_path('routes/api.php'));
        });
    }

So having this namespace customised for our web.php routes file generates this issue of Laravel not detecting that the Inertia route controller from the macro its actually relative and not absolute as it was before.

This could also happen with anything that modifies the namespace like route groups, etc. So I think is better having this as a string.

@reinink
Copy link
Member

reinink commented May 10, 2022

Hey thanks so much for this PR. I just bumped into this myself tonight, so I fixed it (e026fe8), but now I'm realizing that you already submitted a fix 🤦‍♂️

Thanks so much either way for this! I hope to get this released soon.

@reinink reinink closed this May 10, 2022
@d8vjork
Copy link
Author

d8vjork commented May 10, 2022

@reinink Lovely! I was waiting using a forked for my company project, now we can go again to this

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.

2 participants