When only modules are used, links with no presenter mask cannot be created with just one RouteList

[spaze]

Version: dev-master

Bug Description

When only modules are used in an app, links cannot be created when only one RouteList is used. Exception saying "Invalid link: No route for Bar:Foo:waldo()" is thrown.

Steps To Reproduce

1. Download nette/sandbox
2. composer install
3. Move Homepage presenter into a Home module:

-namespace App\Presenters;
+namespace App\HomeModule\Presenters;

4. Create another module with another presenter (and put it into FooPresenter.php for example):

namespace App\BarModule\Presenters;
final class FooPresenter extends \App\Presenters\BasePresenter
{
	public function renderWaldo(): void
...

5. Update RouterFactory to read

		$router->addRoute('foobar/<action>', [
			'presenter' => 'Foo',
			'action' => 'default',
			'module' => 'Bar',
		]);
		$router->addRoute('<presenter>/<action>', [
			'presenter' => 'Homepage',
			'action' => 'default',
			'module' => 'Home',
		]);

Note the 'module' key.

6. Update Homepage/default.latte to include

<a n:href=":Bar:Foo:waldo">bar module</a>

7. Enable debug mode, edit Bootstrap.php, e.g.:

-		//$configurator->setDebugMode('23.75.345.200'); // enable for your remote IP
+		$configurator->setDebugMode(true); // enable for your remote IP

8. Start PHP (see the help in nette/sandbox), and load the page

9. You should see

User Warning
Invalid link: No route for Bar:Foo:waldo()

10. Change the router (RouterFactory.php) code:

-		$router->addRoute('foobar/<action>', [
-			'presenter' => 'Foo',
-			'action' => 'default',
-			'module' => 'Bar',
-		]);
-		$router->addRoute('<presenter>/<action>', [
-			'presenter' => 'Homepage',
-			'action' => 'default',
-			'module' => 'Home',
-		]);

+		$routerBar = new RouteList('Bar');
+		$routerBar->addRoute('foobar/<action>', [
+			'presenter' => 'Foo',
+			'action' => 'default',
+		]);
+		$router->add($routerBar);
+
+		$routerHome = new RouteList('Home');
+		$routerHome->addRoute('<presenter>/<action>', [
+			'presenter' => 'Homepage',
+			'action' => 'default',
+		]);
+		$router->add($routerHome);

Note that module name was moved from the module key to the RouteList constructor and that now, instead of adding a Route, we add a RouteList.

11. Load the page in your browser again, no error

Expected Behavior

It works :-) Or the exception is more clear or docs are clear.

Possible Solution

There are several options, not sure which one is the correct one, feedback wanted:

• update docs and say when only modules are used then each module needs to use its own RouteList with the module name specified in the constructor
• somehow try to make it work as it used to before nette/routing was created

Spent quite some time debugging the problem and this is what helped to fix (and understand) the problem for me:

#208 describes that nette/routing classes have no idea about //modules// and that Application\Routers\RouteList solves that. But this is in constructUrl:

	public function constructUrl(array $params, Nette\Http\UrlScript $refUrl): ?string
	{
		if ($this->module) {
			if (strncmp($params[self::PRESENTER_KEY], $this->module, strlen($this->module)) === 0) {
				$params[self::PRESENTER_KEY] = substr($params[self::PRESENTER_KEY], strlen($this->module));
			} else {
				return null;
			}
		}

		return parent::constructUrl($params, $refUrl);
	}

so if there's no $this->module then it just calls parent::constructUrl which doesn't know anything about modules.

The docs says:

In Nette we can split presenters into modules. Therefore we need to work with those modules in routes. We can use module parameter in Route class:
+
If we have more routes that we want to group together in a module, we can use RouteList with name of module in constructor:

So I tried using the constructor param and it worked.

Honestly I'd be totally fine with just updating the docs (and maybe add a BC warning to nette/application release notes?), let me know I can fix it (just not sure how).

Thanks and sorry for this long bug report, tried to write down all I've learned in the past 2 days or so.

[spaze]

Here's a failing test https://github.com/spaze/application/commit/acfed5df8097de070bc805e65c117cd399b0e523
(also attaching as a file, remove the .txt extension), it fails with $list = new RouteList;, not with $list = new RouteList('Auth');.

It fails on master, doesn't fail on the 2.4 branch. And seems the problem is triggered by routes that don't specify a presenter mask (<presenter>) but instead would always use the default presenter specified in the metadata array.

[JanMikes]

Hi, might https://forum.nette.org/cs/32297-routovani-upgrade-z-2-4-na-3-0 be related to this issue?

[spaze]

Hi @JanMikes, quite possibly. I've done something similar as what you describe in the forum post, and that was use multiple routelists, one per module (see this commit, the commit message is enough, the diff is specific to my site so it won't help anyone I guess)

[spaze]

Thanks @dg!

[dg]

Sorry for the delay, somehow I missed it.