Prevent comment forms from marking rendered nodes as uncacheable

Well, could we render a single render array of a node and check the max-age / cache contexts of it?

Reverted back the create_placeholder. Need some input how to test that.

Well first up it would be great to see evidence of manual performance testing that this fixing the issue.

Thanks, @sasanikolic!

To test this, you can do something like this, in a kernel test:

// First, ensure some node bundle has a comment field.
$build = NodeViewBuilder::view($node_of_bundle_that_has_a_comment_field);
$this->renderer->renderRoot($build);
$this->assertIdentical(Cache::PERMANENT, $build['#cache']['max-age']);
$this->assertFalse(isset($build['#printed']), 'Cache hit');

I have extended the test, but it fails. Also, together with @berdir, we tried with render and renderRoot. With render, something is changing the max-age from -1 to 0 (appartently in accessCheck), with renderRoot it was false, which makes no sense.

renderRoot won't work, because it renders all placeholders including the comment form (per definition).

So you need to use:

$context = new RenderContext();
$renderer->executeInRenderContext(function($context) use ($renderer, &$build) {
  $renderer->render($build);
});

// etc.

Edit: Disregard - I see what you did there now :).

The above works fine, because what berdir did above there is to use a child element and then check the cacheability of the child element, which is enough.

+++ b/core/modules/system/tests/modules/entity_test/entity_test.module
@@ -622,6 +622,10 @@ function entity_test_entity_prepare_view($entity_type, array $entities, array $d
+  // Only apply to entity_test entity types.
+  if ($entity->getEntityType()->getProvider() != 'entit_test') {
+    return AccessResult::neutral();
+  }

I believe this typo is the reason for the test fails.

Fixing the typo.

Patch looks great, just one nitpick to fix:

+++ b/core/modules/system/tests/modules/entity_test/entity_test.module
@@ -622,6 +622,10 @@ function entity_test_entity_prepare_view($entity_type, array $entities, array $d
+  // Only apply to entity_test entity types.

s/entity types/entities/

Fixed.

Since my only work on the actual code here is a typo fix in #17, the docs typo fix here, and pointing people in the right direction on how to write a regression test, I think it's fine for me to RTBC this.

RTBC + 1 - if tests pass

(please add me to commit credit if deemed okay)

Clarified IS.

We've got tests and fix - this is good to go. Committed 18d4de1 and pushed to 8.0.x. Thanks!