big_pipe_theme_suggestions_big_pipe_interface_preview show Notice for

Thank you for reporting, we will need a test case also showing the issue too.

Changing to current development branch.

Modified small line to add tests cover for the issue.

// Cover some case argument item is an array not string.

Think this needs some work, read it a few times and not sure what it's testing exactly.

Created merge request and split the tests.

Interesting 🤔 But: https://3v4l.org/lsbEX — I cannot reproduce the notice?

What PHP version is this happening in?

Hi @Wim Leers, Thanks for your help to review.

If the arguments has a array item inside it will show the notice.

$variables['arguments'] = ['foo_bar', ['sample']];
var_dump(preg_replace('/[^a-zA-Z0-9]/', '_', $variables['arguments']));

1) Drupal\Tests\big_pipe\Kernel\BigPipeInterfacePreviewThemeSuggestionsTest::testBigPipeThemeHookSuggestions
Array to string conversion
/builds/issue/drupal-3403941/core/modules/big_pipe/big_pipe.module:122
/builds/issue/drupal-3403941/core/modules/big_pipe/tests/src/Kernel/BigPipeInterfacePreviewThemeSuggestionsTest.php:95
/builds/issue/drupal-3403941/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
ERRORS!
Tests: 1, Assertions: 1, Errors: 1.

Test-only results btw.

D'oh, of course 😄 Thanks for kindly explaining that, @phthlaap! 😊

This is a bug fix for ✨ Interface previews/skeleton screens through optional "preview" or "placeholder" templates Fixed .

… and that made me realize: this should not ever be possible. Because lazy builders only allow scalar values.

You should be getting this exception:

A #lazy_builder callback's context may only contain scalar values or NULL.

Because $variables['arguments'] comes from

  protected static function createBigPipeJsPlaceholder($original_placeholder, array $placeholder_render_array) {
    $big_pipe_placeholder_id = static::generateBigPipePlaceholderId($original_placeholder, $placeholder_render_array);

    $interface_preview = [];
    if (isset($placeholder_render_array['#lazy_builder'])) {
      $interface_preview = [
        '#theme' => 'big_pipe_interface_preview',
        '#callback' => $placeholder_render_array['#lazy_builder'][0],
        '#arguments' => $placeholder_render_array['#lazy_builder'][1],
      ];
…

See the test at \Drupal\Tests\Core\Render\RendererPlaceholdersTest::testNonScalarLazybuilderCallbackContext().

So the test coverage is still missing one thing: how can some lazy builder cause big_pipe_theme_suggestions_big_pipe_interface_preview() to hit this problem, but not trigger that exception? 🤔

Is big_pipe_theme_suggestions_big_pipe_interface_preview() somehow executed before

      assert(count($elements['#lazy_builder'][1]) === count(array_filter($elements['#lazy_builder'][1], function ($v) {
        return is_null($v) || is_scalar($v);
      })), "A #lazy_builder callback's context may only contain scalar values or NULL.");

maybe?

Or much simpler: perhaps you're developing this with assertions turned off? 🤔 Can you try to see if you can reproduce the originally reported problem when assertions are enabled? 🙏

Then the proper solution is still not to do the proposed change, but instead convert that assert() to an if (…) { throw new \LogicException(); } .

If my analysis is wrong, it should be possible to disprove it with a test.