_content controllers may only return render arrays, not strings

Will some other issue add the exception when a controller returns a string?

Yes I was expecting to see the exception too

1. +++ b/core/lib/Drupal/Core/Controller/HtmlControllerBase.php
   @@ -49,20 +49,26 @@ public function __construct(TitleResolverInterface $title_resolver, RenderHtmlRe
   +   * @param array|\Drupal\Core\Page\HtmlFragment $page_content
   ...
        if ($page_content instanceof HtmlFragment || $page_content instanceof Response) {
   

   Should this also contain Response as a potential type?

2. +++ b/core/lib/Drupal/Core/Controller/HtmlControllerBase.php
   @@ -49,20 +49,26 @@ public function __construct(TitleResolverInterface $title_resolver, RenderHtmlRe
   +   * @throws \LogicException
   ...
   +    if (is_string($page_content)) {
   +      throw new \LogicException('_content controllers may not return strings.');
   

   No reason to hold the patch up, but this is a perfect usage for \InvalidArgumentException

+   *
+   * @throws \LogicException

No longer true.

Otherwise looks good.

Are we sure there is no documentation at all in Drupal core which documents this?

fixes #10, reviewed again - agree with @moshe - +1 rtbc

Let's actually provide something helpful.

No concerns from me.

Committed 961f188 and pushed to 8.0.x. Thanks!

+++ b/core/modules/system/src/Tests/Routing/RouterTest.php
@@ -44,7 +44,7 @@ public function testDefaultController() {
-    // a page inception style.  This test verifies that is not happening.
+    // a page inception style.  This test verifies that is not happening

Unrelated and doesn't fix the code style bug there anyway - just makes it worse. Removed from the patch.

I've written a concern elsewhere but it's puzzling how this is a DX improvement, where before if you're doing a test you could just provide a string which is the end result response to the browser. And now we must wrap it to get it back to a string through a superfluous drupal_render call.

I'm probably missing things, sorry I didn't get a chance to ask before it was committed.

Other than the type consistency what are we gaining from this?

RTBC + 1 (in retrospective)

I'll add another: discoverability. When you can just return a string, you do so. And then you have less opportunity to learn that should not be theming stuff in your callback, but rather deferring theming via #theme. And then you learn that you should use #attached instead of adding css/js/library globally. And then you learn that you can render cache expensive things, with great control over cache keys, cache tags, expire time, bin, etc.

Thanks for the responses @Wim Leers and @moshe weitzman, just something I'll need to get used to a bit.

I liked the magic when it gets me from Point A to Point B with little resistance. I relate it to inline templating: it's really not something you should be doing, but for rapid prototyping it will get you there faster, then you can work on splitting up the concerns into the template.html.twig and #theme definitions once you know your idea has wings.

@Wim Leers yes although I seem to be in the 1% minority... but it's important to me so I thought I'd speak up.

For workflow: I go with sanity test/minimal viable code: "Can I print something to the browser?" is a good start. Massage it till it does what I want. Then refactor a few times to add things #cache, #attached, #theme and all the other things that require extra files or configuration that are not needed at the beginning "hello world".

return ['#markup'=> $string] feels unnecessary when what my end goal is to

print $string;

(Purposely changed that to print because that is what I did in 6/7 for a blank viewport on hook_menu, though return from the controller is fine).

@Wim Leers oh thanks for clarifying the Response() bit I should know that actually, not sure why that didn't cross my mind... I'll likely use that in my little sanity checks:)

I referenced this stackoverflow question before in the other issue discussing this:
http://stackoverflow.com/questions/553936/in-mvc-how-do-i-return-a-strin...

For the likely "Why doesn't it just work like this" type of question I'm expecting. The snippet is MVC.net, although they are similar concepts. new Response() will be Content() but you still can if you want pass a "string" from the controller's action in MVC.net though most likely you'd use a View() in MVC.net just like you'd use ['#theme' => 'my_theme_hook'] here.