This is part of #2046881: [meta] Avoid "early rendered" strings where beneficial to do so, build structured data to pass to drupal_render() once instead.

The whole update_results_page() function concatenates a giant string, including a call to drupal_render() before returning the big string.

This concatenation includes markup that could be represented with #theme 'item_list', #theme_wrappers 'container'...

Once it's built up, the string is returned. The only place this function is called is in update.php, which calls drupal_render() on the maintenance page.

The maintenance page contains {{ content }} (which contains $output).

We don't need update_results_page() to be an early render, it could simply return structured data.

Comments

thedavidmeister’s picture

thedavidmeister’s picture

Issue summary: View changes
Status: Postponed » Needs work

no longer blocked.

george.echim’s picture

I tried to refactor the update_results_page() function but I don't find any information about how Drupal 8 is handling the ?q= type of url if Clean Urls are not supported.
I found an issue here

thedavidmeister’s picture

The purpose of this issue is not to broadly refactor this function.

Very specifically we want to convert string concatenation that looks like this:

  if (settings()->get('update_free_access')) {
    $output .= "<p><strong>Reminder: don't forget to set the \$settings[&#039;update_free_access&#039;] value in your settings.php file back to FALSE.</strong></p>";
  }

  $links = array(
    '#theme' => 'links',
    '#links' => update_helpful_links(),
  );
  $output .= drupal_render($links);

To things that look more like this:


  $build = array();
  if (settings()->get('update_free_access')) {
    $build['update_free_access_reminder']['#markup'] = "<p><strong>Reminder: don't forget to set the \$settings[&#039;update_free_access&#039;] value in your settings.php file back to FALSE.</strong></p>";
  }

  $build['update_helpful_links'] = array(
    '#theme' => 'links',
    '#links' => update_helpful_links(),
  );

But we don't want to change any logic, just the way strings are built from data.

andypost’s picture

Status: Needs work » Closed (cannot reproduce)

The code now exactly like was proposed

    if (Settings::get('update_free_access')) {
      $message .= '<p>' . $this->t("<strong>Reminder: don't forget to set the \$settings[&#039;update_free_access&#039;] value in your settings.php file back to FALSE.</strong>")  . '</p>';
    }

    $build['message'] = array(
      '#markup' => $message,
    );
    $build['links'] = array(
      '#theme' => 'links',
      '#links' => $this->helpfulLinks(),
    );