tighten / ziggy

Use your Laravel routes in JavaScript.
MIT License
3.91k stars 248 forks source link

regression in javascript : route().current() returns undefined in 1.4.4 for some routes #534

Closed coclav closed 2 years ago

coclav commented 2 years ago

Ziggy version

1.4.4

Laravel version

latest (9)

Description

on front end we user route().current() a lot to highlight active menus. realised this didn't work for most routes since last update of ziggy (1.4.4). route().current() returns undefined

downgraded to 1.4.3 ( composer require tightenco/ziggy@1.4.3 ) and route().current() is back to returning the current route in all situation.

I have 2 browser windows opened, one where i run ziggy 1.4.3 and the other 1.4.4 there is no other difference in the code base whatsoever

URL : http://localhost:8000/wp-spinnaker/processes

ziggy 1.4.3. : image

ziggy 1.4.4 : image

we ahve a lot of routes image

Ziggy call and context

see above

regarding confi below, quantify / milestone / etc. are used in "siblings" route. we have A LOT of routes ( see third image above, cannot paste here)

Ziggy configuration

{
url: 'http://localhost:8000', 
defaults: {},
port: 8000,
routes {
    "uri": "{workspace}/processes",
    "methods": [
        "GET",
        "HEAD"
    ],
    "wheres": {
        "provider": "[a-zA-Z]+",
        "milestone": "active|current|latest|published|awaiting|s\\d+|v\\d+|\\d+",
        "workspace": "^(?!storage|library|api|nova-api|shared|horizon|vapor-ui|dashboard).*$",
        "boxCode": ".*",
        "quantify": "overview|duration|costs|roles|iterations|fte|activities|customfields|settings"
    },
    "bindings": {
        "workspace": "id"
    }
}
}

Route definition

<?php

use Illuminate\Support\Facades\Route;

Route::group(['as' => 'web.'], function () {

    // ... 

    Route::group(
        [
            'middleware' => ['auth', 'auth.requirements'],
        ],
        function () {

            // ...

            Route::group(
                [
                    'prefix' => '{workspace}',
                    'as' => 'workspaces.',
                    ],
                function () {

                    // ...

                    Route::group(
                        [
                            'middleware' => ['workspace-must-be-active'],
                        ],
                        function () {

                            // ...

                            Route::get('processes', 'ProcessController@index')
                                ->middleware(
                                    'roles:member|editor|admin|technicalSupport'
                                )
                                ->name('processes.index');

                            // ...
                        }
                    );
                }
            );
        }
    );
});
bakerkretzmar commented 2 years ago

I can reproduce this and I'll push up a fix shortly. This issue is caused by your workspace 'where' condition including the ^ and $ tokens, those match the very beginning and end of the string, not just the beginning and end of that route parameter.

You should be able to remove those and see everything work normally again—please let me know if that doesn't fix it.

- ^(?!storage|library|api|nova-api|shared|horizon|vapor-ui|dashboard).*$
+  (?!storage|library|api|nova-api|shared|horizon|vapor-ui|dashboard).*

Including ^ and $ has no effect on your routes, Laravel (Symfony actually) strips them off internally, probably to avoid this very issue: https://github.com/symfony/symfony/blob/6.1/src/Symfony/Component/Routing/Route.php#L431-L446

One other thing to note here is that your workspace pattern will also match forward slashes, which might not be what you want. The Laravel docs (Routing > Encoded Forward Slashes) note that wildcard .* patterns are only supported in the very last parameter. In my local testing, /wp-/spin/naker/processes loaded the web.workspaces.processes.index route despite all those extra slashes.

Your pattern for that segment should actually probably just be (?!storage|library|api|nova-api|shared|horizon|vapor-ui|dashboard), or maybe something like (?!storage|library|api|nova-api|shared|horizon|vapor-ui|dashboard)[\w\d_\-]* if you really want to avoid matching values that event just start with storage, library, etc.