Macro localizedUrl : Call to a member function parameters() on null

[Okipa]

Hi @ivanvermeyen,

The Call to a member function parameters() on null error message is returned when I enter a random URL in order to trigger a 404 error.

It seems that the Route::current returns null and generate an error in the macro. Is this something you get too on your projects ?

[ivanvermeyen]

Hello,

Thanks for reporting this. I added this feature only recently so I haven't extensively used it yet in a real project. But I'm trying to reproduce it.

Where are you calling Route::localizedUrl() in your project? Is your 404 page using a master layout that has that call? Or are you using a view composer?

I'll try to put this under test.

[Okipa]

Thanks for your answer.

There are two cases that are returning errors.

Usage in a service provider

I use a service provider in order to generate multilingual meta tags (with https://github.com/artesaos/seotools).

Here is my usage :

class SeoServiceProvider extends ServiceProvider
{
    /**
     * Register bindings in the container.
     *
     * @return void
     * @throws \Exception
     */
    public function boot()
    {
        foreach (supportedLocaleKeys() as $localeKey) {
            SEO::metatags()->addAlternateLanguage($localeKey, Route::localizedUrl($localeKey));
        }
     }
}

Error returned : Call to a member function parameters() on null on Route::current()->parameters() in your macro.

Usage in a language switcher

I built a language switcher component : https://github.com/Okipa/starter/blob/multilingual/resources/views/components/common/multilingual/lang-switcher.blade.php

When I type a wrong URL in my browser like <base-url>/wrong, the same Call to a member function parameters() on null error is returned at the same place.

EDIT

Do you think this is related to the usage of a macro ? The Route::current() seems to return null, I wonder if there would be a way to get it in another way.

[ivanvermeyen]

I got the same issue with view composer etc as well. I suppose the reason is because there is no actual Route registered for a 404. Which kinda makes sense.

So I can catch this in the macro if I change it like this:

Route::macro('localizedUrl', function ($locale = null, $parameters = null, $absolute = true) {
    if ( ! $route = Route::current()) {
        // return the current unmodified url ?
    }

    $parameters = $parameters ?: $route->parameters();
    $model = Collection::make($parameters)->first();

    if ($model instanceof ProvidesRouteParameters) {
        $parameters = $model->getRouteParameters($locale);
    }

    if (is_callable($parameters)) {
        $parameters = $parameters($locale);
    }

    return route($route->getName(), $parameters, $absolute, $locale);
});

Now the only thing to decide is what to return... the current unmodified url or an empty string or null or...?

[ivanvermeyen]

I suppose it doesn't really matter what it returns, it probably won't be used anyway. So I'll stick with URL::current() for now.

In very special cases you could always do a check if Route::current() exists before calling the macro, but I doubt that will ever be an issue.

[Okipa]

@ivanvermeyen
Wouldn't be interesting to return the current URL but prefixed with the locale ?

EDIT

Or not, according to the omit_url_prefix_for_locale value.

I don't really know what is the ideal use case here, I'm testing your solution right know.

[ivanvermeyen]

Yeah, there's also the option to use custom (sub)domains or change the position of the locale slug. Since there is no actual route, I don't think the intended URL can be guessed very easily...

If needed I could explore this further tho.

[Okipa]

For example, with the following configuration :

• languages : fr, en
• omit_url_prefix_for_locale : 'fr'
• URL : <base-url>/wrong

We could have :

• FR : <base-url>/test
• EN : <base-url>/en/test

I would at least allow to switch language, even on a 404 page, in order to get it translated.

[ivanvermeyen]

I'll check it out 👍

[Okipa]

Thanks for your time !

[Okipa]

This feature could also be used for the alternate meta that should return the translated URL with the given locale.

As I mentioned it here : https://github.com/codezero-be/laravel-localized-routes/issues/16#issuecomment-570554273, in a service provider, the translated URL is not properly generated with your solutions :/

[ivanvermeyen]

I'm always a bit blurry about the order in which everything triggers in a Laravel request cycle. When I dd(\Route::current()); in the ServiceProviders' boot() method, it returns null. And dd(\Route::getRoutes()); doesn't show the routes I have registered. So I suppose routes haven't been registered yet at that point?

Maybe you can create a middleware to add it to the SEO?

[Okipa]

That's exactly what I've just done and I was coming here to tell you when I saw your answer ;)

It seems that the service provider is not a good place to play with routes, they are not fully loaded yet.

[ivanvermeyen]

I came across this page while looking for a way to test a custom 404 page with Laravel/PHPUnit. https://laraveldaily.com/route-fallback-if-no-other-route-is-matched/

This points out another complication:

"Another side effect of this: instead of just default 404 page, this Route::fallback() will follow all the Middlewares in ‘web’ group, from app/Http/Kernel.php"

So I tried this, and indeed, the middleware doesn't run for a 404 page unless you register something like:

Route::fallback(function () {
    return view('errors.404');
});

I'm also not entirely sure if Route::fallback() is only used for 404 pages or also for something else.

I will dig a bit deeper into this, but it may take a while :)

[Okipa]

@ivanvermeyen
I'm currently digging this too, I found out that the only way to get the web middleware stack be used with a 404 page is to use this (it does exist since 5.5 but I was not aware of this https://laravel-news.com/laravel-v5-5-5).

I think that it only take care about the 404, because the fallback is only triggered when no route matches the URL.

I made this work by changing a bit your macro again because the Route::currentRouteName()is not defined.

        Route::macro('localizedUrl', function ($locale = null, $parameters = null, $absolute = true) {
            if (! Route::current() || ! Route::currentRouteName()) {
                return \URL::current();
            }
            $parameters = $parameters ?: Route::current()->parameters();
            $model = Collection::make($parameters)->first();
            if ($model instanceof ProvidesRouteParameters) {
                $parameters = $model->getRouteParameters($locale);
            }
            if (is_callable($parameters)) {
                $parameters = $parameters($locale);
            }

            return route(Route::currentRouteName(), $parameters, $absolute, $locale);
        });

[ivanvermeyen]

These 2 tests are passing without changing the latest dev-master version:

/** @test */
public function it_returns_the_current_url_for_existing_non_localized_routes()
{
    $this->setSupportedLocales(['en', 'nl']);

    Route::get('non/localized/route', function () {
        return [
            'current' => Route::localizedUrl(),
            'en' => Route::localizedUrl('en'),
            'nl' => Route::localizedUrl('nl'),
        ];
    })->name('non.localized.route');

    $response = $this->call('GET', '/non/localized/route');
    $response->assertOk();
    $this->assertEquals([
        'current' => url('/non/localized/route'),
        'en' => url('/non/localized/route'),
        'nl' => url('/non/localized/route'),
    ], $response->original);
}

/** @test */
public function it_returns_localized_urls_for_non_existing_routes_that_have_a_supported_locale_in_their_url()
{
    $this->setSupportedLocales(['en', 'nl']);

    Route::localized(function () {
        Route::fallback(function () {
            return response([
                'current' => Route::localizedUrl(),
                'en' => Route::localizedUrl('en'),
                'nl' => Route::localizedUrl('nl'),
            ], 404); //=> By default, the fallback returns a 200 status and not a 404...
        })->name('404');
    });

    $response = $this->call('GET', '/nl/non/existing/route');
    $response->assertNotFound();
    $this->assertEquals([
        'current' => url('/nl/non/existing/route'),
        'en' => url('/en/non/existing/route'),
        'nl' => url('/nl/non/existing/route'),
    ], $response->original);
}

In the second test I registered the fallback, but it would be nice to include it in the package while still keeping it easily configurable.

Perhaps I can add a config option to use_localized_404_page and set a localized_404_action like Custom404Controller@action, which could default to the Laravel 404 page (if I can find it).

The next step would be to look at non existing routes that don't have a supported locale in the URL. Those would need their own fallback and I'll still have to figure out how to generate non existing localized URL's :)

[Okipa]

The default error page is handled here : https://github.com/laravel/framework/blob/6.x/src/Illuminate/Foundation/Exceptions/Handler.php#L399.

IMO the package role is not necessarily to handle such things like custom 404 feature, but I would need to think again about that with a fresh mind, I had a long day and my brain can't think anymore !

Anyway, your last point may be quite important to handle indeed.

[ivanvermeyen]

Just FYI: found a great example of the 404 behavior on a big webshop site.

There, you can switch language on a 404 page with supported locale in the URL, like /nl/asdf.

On other faulty URL's, you get redirected to to the localized version, so /meh/asdf gets redirected to /nl/meh/asdf and there you can switch to /en/meh/asdf.

https://www.coolblue.be/meh/asdf

[Okipa]

It seems to be a logical behaviour, despite the fact the URL is quite strange /en/switchlanguage?return=/en/meh/asdf).

If I take example on another website I did with the https://github.com/mcamara/laravel-localization package at the time, the behaviour is the same : http://www.gites-belves.fr/en/wrong

I think that if your package can reproduce this behaviour, it will provide a complete solution and stay much lighter than the mcamara package : yours is closer to the Laravel standards, which I appreciate a lot.

I thought again about your idea to implement a solution for a custom 404 and the more I think about it, the more I'm convinced that you should keep your package light and let users implement it if they wish to.

But IMO the package should at least be able to generate a Route::localizedUrl($locale) with any URL, even if it is not a declared route, a named route, ...

We may gain time by spying a bit on this : https://github.com/mcamara/laravel-localization/blob/master/src/Mcamara/LaravelLocalization/LaravelLocalization.php#L885. This is the way mcamara provides a way to generate a localized URL and I think he had to deal with the same problematic we have now ;)

[ivanvermeyen]

I agree. The fallback is basically a catch all and people may want to implement their own catch all for a different purpose. It's also easy enough to add this functionality manually by registering the fallback route. So it is probably sufficient to provide instructions in the readme to set this up.

I will work out the URL generation and add tests for all possible scenarios.

Thanks for thinking this through with me, it really helps to work out the feature and make better decisions. 👍

[ivanvermeyen]

Update:

I just pushed a bunch of code that should localize any 404 URL and a few tests to prove it's working. It should be working when omitting a main locale slug, or when using custom domains too.

The generateFromUrl method is a huge mess, so I will be refactoring this greatly to make it more readable.

I may also need to add support for query strings and hash tags in the URL... Or think about if it's needed or not for 404's...

I intend to provide instructions in the README on how to add a fallback route to localize 404 pages. To avoid problems with other catch all routes, I decided to require a name of 404 for these 404 fallbacks.

Feel free to review the changes and suggest any changes... :)

[Okipa]

Nice work, seems good !

I'll test this in real conditions tomorrow morning and I let you know if I see any Issue or behaviours that still could be improved.

You're welcome for the help, it is always easier to find the good way to achieve features when we are several brains thinking on it ;)

I also maintain several packages and I sometime spend more time to think about the good way to resolve a use case and how to architecture it than actually developing it !

[Okipa]

I've made a few test with your current dev-master branch and it seems to be good for almost all previously detected problems :

• No error on a 404 page (with or without fallback treatment)
• The URLs are correctly localized, event if they do not exist

On the other hand, with or without the fallback treatment, the URL are correctly translated by the localizedUrl but an english url (for example en/wrong) still not triggers the EN translation of the app : app()->getLocale() returns the default configured locale, not en.

[Okipa]

It seems that your SetLocale middleware is not triggered with an en/wrong 404 URL.
I am looking for the reason for that.

EDIT

That was a dumb error : the fallback was not triggering the middleware and was of course not placed into the Route::localized group...

Anyway, I fixed that and $locale = $request->route()->getAction('localized-routes-locale'); in the SetLocale does now returns null.

Here my fallback treatment :

Route::fallback(function(){
   return response()->view('errors.404'), 404);
})->middleware(\CodeZero\LocalizedRoutes\Middleware\SetLocale::class)->name('404');

[Okipa]

One more update : It seems that this is related to the fact I activated omit_url_prefix_for_locale for fr.

When I switch for the en locale on the 404, the EN translation is correctly activated, but when I want to switch back to FR, the locale is not found, due to the omit_url_prefix_for_locale activation for this locale.

It also seems that the whole treatment is working well (except the case above) without any fallback use. It may be not usefull to declare one.

EDIT

I did test it with an entity which I call with /page/{slug}. If I test with /page/wrong, there the 404 is correctly switchable from a language to another without problem.

My problem remains only when I trigger the 404 from the base URL (/wrong), there the app does not switch to another language, even if the URL is /en/wrong.

[ivanvermeyen]

Thanks for testing.

After reproducing your use case (and discovering a related issue) here's what's going on: if the fallback is not in the Route::localized() function, the default app locale will always be used. Unless you set use_localizer configuration option to true.

In the following test, I changed the app locale to en to make it fail. Then enabling the localizer setting and adding the middleware to the fallback to make it pass again:

/** @test */
public function it_returns_a_localized_url_for_a_non_localized_fallback_route_if_the_url_contains_a_supported_locale()
{
    $this->setSupportedLocales(['en', 'nl']);
    $this->setUseLocalizer(true);
    $this->setAppLocale('en');

    Route::fallback(function () {
        return response([
            'current' => Route::localizedUrl(),
            'en' => Route::localizedUrl('en'),
            'nl' => Route::localizedUrl('nl'),
        ], 404);
    })->middleware(\CodeZero\LocalizedRoutes\Middleware\SetLocale::class)->name('404');

    $response = $this->call('GET', '/nl/non/existing/route');
    $response->assertNotFound();
    $this->assertEquals([
        'current' => url('/nl/non/existing/route'),
        'en' => url('/en/non/existing/route'),
        'nl' => url('/nl/non/existing/route'),
    ], $response->original);
}

Now this still doesn't fix this use case. I made this test fail by setting the app locale to en, different than the nl omit locale:

/** @test */
public function it_returns_a_localized_url_for_a_non_localized_fallback_route_when_omitting_the_main_locale()
{
    $this->setSupportedLocales(['en', 'nl']);
    $this->setOmitUrlPrefixForLocale('nl');
    $this->setUseLocalizer(true);
    $this->setAppLocale('en');

    Route::fallback(function () {
        return response([
            'current' => Route::localizedUrl(),
            'en' => Route::localizedUrl('en'),
            'nl' => Route::localizedUrl('nl'),
        ], 404);
    })->middleware(\CodeZero\LocalizedRoutes\Middleware\SetLocale::class)->name('404');

    $response = $this->call('GET', '/non/existing/route');
    $response->assertNotFound();
    $this->assertEquals([
        'current' => url('/non/existing/route'),
        'en' => url('/en/non/existing/route'),
        'nl' => url('/non/existing/route'),
    ], $response->original);
}

The reason:

1. the fallback is not localized
2. my codezero/localizer package then tries to figure out the locale by looking at the URL, but it doesn't find any supported locale, so it defaults to the app locale.

Will now look into fixing it.

[ivanvermeyen]

Ok, so the question is (thinking out loud)...

If the fallback route isn't in Route::localized(), do we bother to detect locale in the URL? Or do we require the developer to add a localized fallback for this?

If we would make this work, is there any other need to add localized fallbacks? It would be nice if 1 global fallback would be enough.

If we want to make a non localized fallback work, we need to:

• detect if it's a fallback
• detect any supported locale in the url
• detect if locale can be omitted
• send any captured locale to the LocaleHandler in the middleware

I will continue to work this out (shouldn't be too hard) and see where it gets us.

[ivanvermeyen]

Tests are at green. Don't even need to enable use_localizer now, unless you want the other detection features that it offers.

/** @test */
public function it_sets_the_right_locale_when_accessing_non_localized_fallback_routes_with_omitted_prefix()
{
    $this->setSupportedLocales(['en', 'nl']);
    $this->setOmitUrlPrefixForLocale('nl');
    $this->setAppLocale('en');

    Route::fallback(function () {
        return App::getLocale();
    })->middleware(SetLocale::class);

    $response = $this->call('GET', '/non/existing/route');
    $response->assertOk();
    $this->assertEquals('nl', $response->original);

    $response = $this->call('GET', '/en/non/existing/route');
    $response->assertOk();
    $this->assertEquals('en', $response->original);
}

Got some duplication going on and I feel my code is getting a bit scattered, so refactor inc. ;)

[Okipa]

Hi @ivanvermeyen, I just made some tests and everything seems to be working just fine !

As specified, in order to get the app locale updated from the URL in a 404, we have to declare a route fallback named 404 and using the SetLocale middleware, you'll indeed just have to specify this in the doc.

The error pages customization official doc is here : https://laravel.com/docs/5.8/errors#custom-http-error-pages

Route::fallback(function () {
    return response()->view('errors.404', 404);
})->middleware(SetLocale::class)->name('404');

I did not identified any other tricky use cases which would need to be investigated so I suppose that you can release this when you're ready.

Thanks for you time and your good work, we'll start to use your package in production soon and there's no better way to check if everything is OK or if we missed something ;)

[ivanvermeyen]

There is one more issue I need to tackle and that is handling the scenario where people don't name there routes and they call `Route::localizedUrl().

That's because I'm calling the route() helper to automatically generate the URL. The only way to get around is is to manually replace any slug or custom domain and then call the url() helper instead, passing through any parameters.

[Okipa]

This is would indeed be great to handle that case too.

If that represents a significant amount of work, you also could delay this and specify in the documentation that the routes currently have to be named to work with this macro, at least during the feature development.

Then you could release the new feature with a new tag. IMO, it could be easier to follow the package developments when they are released with small changes. It also would make sense as this is a new feature, unlike what you've been fixing here, which was a malfunction.

But that's your call ! You maybe already did the work ;)

[ivanvermeyen]

I think this already does most of the work, replacing the locale bits in the URL:

https://github.com/codezero-be/laravel-localized-routes/blob/e28501de0456e00175f7d349a230ed83c8279894/src/LocalizedUrlGenerator.php#L105-L123

Just need to get the original path (uri/with/{placeholders}) and take care of the parameters. So might as well fix it now I think.

[Okipa]

Great !

[ivanvermeyen]

I haven't used the url() helper much, but it seems to behave completely different than the route() helper when it comes to parameters...

$url = url('dummy/{test}/{url}', ['test' => 'hello', 'url' => 'world']);

Should return http://localhost//dummy/hello/world in my mind, but it return this instead:

http://localhost/dummy/{test}/{url}/hello/world

Am I missing something?

Thinking if there's another simple solution for ths...

[ivanvermeyen]

Tests are passing! 🎉

Not using the url() helper anymore. I think we got everything covered so far, so I'll update the docs and tag a release. 👌

[Okipa]

Nice, well done !

Looking forward your next release 🚀

[ivanvermeyen]

I'm about to release 2.2.0. There were some more 404/fallback edge cases that I fixed.

It would simplify things a lot if we could run the middleware somewhere else in the app, so it would also trigger on 404's. If that is possible, we can maybe even ditch the whole fallback thing.

[Okipa]

I was thinking about that possibility too : the only way seems to add it into the main protected $middleware group, but it does not make sense because it will be triggered even for API requests...

And to trigger the web middleware group, you also have to pass through the fallback process. So, I don't think this is possible but I may miss something.

[ivanvermeyen]

I'll continue my research on this while I restructure the current codebase now that tests are all passing. Thanks again for your input. 👍

[Okipa]

You're welcome, thanks for your investigations and fixes too !

[ivanvermeyen]

Interesting tips about using the fallback route, especially this part:

Using abort(404) and ModelNotFound exceptions

https://themsaid.com/laravel-55-better-404-response-20170921

[Okipa]

Interesting indeed, at least to avoid api request to return html when a fallback is declared. Will not use the other features right now but will keep this in my mind for later !