Problem/Motivation

Usage of the theme() function directly invokes the theme layer and markup rendering directly prior to other calls and alterations that may be necessary to adjust the resulting markup sent to the user agent.

Moving to render arrays from D6 to D7 provided an abstraction of how markup would eventually be constructed so that it could be extended and altered throughout page execution before finally invoking a theme callback. In order for full compliance, any usage of theme() returning markup should instead return renderable arrays.

Much of this conversion didn't fully take place in during development from D6 to D7. Given our effort to refactor how Render API operates, we need to start with a comprehensive usage of drupal_render().

Proposed resolution

Only drupal_render() should invoke theme(). Convert all calls to theme() to renderable arrays which are then rendered via drupal_render().

  // Old:
  $output = theme('foo', array('bar' => $bar));
  return $output;

  // New:
  $foo = array(
    '#theme' => 'foo',
    '#bar' => $bar,
  );
  return drupal_render($foo);

Reviewer's/writer's notes

Do NOT attempt to do this (you'll get a fatal error when drupal_render() tries to modify your array by reference):

  $output = drupal_render(array('#theme' => 'foo', '#bar' => $bar));

Always create the renderable array and then render it in two steps.

If presented with something like this:

function theme_image_crop_summary($variables) {
  return theme('image_resize_summary', $variables);
}

then be explicit with the renderable array and do this:

function theme_image_crop_summary($variables) {
  // #theme image_resize_summary expects 'foo' and 'bar'.
  $image_resize_summary = array(
    '#theme' => 'image_resize_summary',
    '#foo' => $variables['foo'],
    '#bar' => $variables['bar'],
  );
  return drupal_render($image_resize_summary);
}

do NOT do something like this:

foreach($variables as $key => $variable) {
  $variables["#$key"] = $variable;
}

Other general guidelines

  • In Drupal 8 the l() function calls drupal_render() on the first parameter passed to it if that parameter is an array. This means $foo = ('#markup' => 'bar'); l(drupal_render($foo), $path, $options); is superfluous, the drupal_render() inline with l() should be avoided here, ie. use l($foo, $path, $options); in this case.
  • The name of the variable for the renderable array should be the same as '#theme', (ie. $foo = array('#theme' => 'foo', ... );) if/when this would conflict with an existing variable in the current scope, $build is the alternative.
  • Don't try to do anything new/fancy like returning the renderable array instead of rendering it where theme() was, despite this probably being "better" in many cases it will also incur quite a bit of extra testing overhead. We're trying to minimise the scope of this issue. Don't change the name/structure of any existing variables or try to add/remove/merge any existing functions. Touch nothing but the lamp (@see Aladdin). If the patch does change anything else, it will need manual testing and therefore slow/stall what should be relatively simple tasks. @thedavidmeister, who wrote this rule, is aware of the irony/hypocrisy in that the patches he submitted (before writing this rule) all refactored things slightly when they probably shouldn't have :P
  • If the conversion is a simple old -> new as above with no other refactor, a careful review should be enough for RTBC. Anything more will require manual testing. Anything sitting behind AJAX, like the views UI requires manual testing too as the testbots can't see it.
  • There is a Drupal.theme function being called in JavaScript files but this is not related to this conversion.

Remaining tasks

Fixed, yay!

Good to go

  • EMPTY!!!

Has patch, needs review

  • EMPTY!!!

Needs patch/further work

  • EMPTY!!!

Postponed temporarily

    Calls to theme()

    https://api.drupal.org/api/drupal/core%21includes%21theme.inc/function/c...

    User interface changes

    None :)

    API changes

    None :)

    Original report by @thedavidmeister

    There's a bunch of discussion around what we can do to make rendering and the theme system better. Currently there are ideas blocked simply because sometimes theme() is called inside drupal_render() and sometimes it isn't. [...] We should just cleanup what direct theme() calls are left in core so it doesn't hold back refactors elsewhere

    Comments

    c4rl’s picture

    $> grep -r " theme(" . | wc -l
    358
    

    This should be fun. :)

    thedavidmeister’s picture

    yeah... not sure how we want to organise this one. I guess just dive in and see where we can find patterns in the chaos?

    thedavidmeister’s picture

    Assigned: Unassigned » thedavidmeister

    i'm going in..

    thedavidmeister’s picture

    Title: Don't call theme() directly anywhere outside drupal_render() » [meta] Don't call theme() directly anywhere outside drupal_render()
    Assigned: thedavidmeister » Unassigned

    I think by module/theme is the way to go. I'll start adding remaining tasks as I roll patches.

    c4rl’s picture

    I'll also add some tasks as well.

    andypost’s picture

    Issue tags: +Novice, +clean-up

    there could be a lot of novice issues for sprint

    andypost’s picture

    Issue summary: View changes

    Denote that the remaining task list isn't comprehensive.

    stevector’s picture

    I added a link to https://api.drupal.org/api/drupal/core%21includes%21theme.inc/function/c... which has the number of calls to theme() as 192.

    thedavidmeister’s picture

    #7 - you're right, the grep for theme(' provides an overestimate, while I've been looking through core I've seen that it matches documentation, some javascript files and some debug code as well as actual theme() calls.

    thedavidmeister’s picture

    Issue summary: View changes

    Adding a link to api.drupal.org

    thedavidmeister’s picture

    ok, issues are up. I think I've found them all but I'll definitely give it all another sweep once we get through these.

    Samvel’s picture

    What to do, if we have this situation?

    function theme_image_crop_summary($variables) {
      return theme('image_resize_summary', $variables);
    }
    
    andypost’s picture

    @Samvel suppose better remove useless functions so introduce one theme_image_effect_summary() and convert similar calls to new function

    Cottser’s picture

    Issue summary: View changes

    Simplify conversion instructions after IRC discussion

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    #11 - I'm not sure I agree with that. We're not looking to add/remove any functions here, just get things routing through drupal_render() and staying functionally equivalent otherwise.

    I don't know if this is the right thing either but I'd say have a look at what theme_image_resize_summary needs and convert based on that:

    say $variables looks like this:

    $variables = array('foo' => 'one', 'bar' => 'two', 'baz' => 'three');
    

    and theme_image_resize_summary expects $variables('foo' = 'somedefault', 'bar' = 'anotherdefault').

    Then do this, I suppose:

    function theme_image_crop_summary($variables) {
      // #theme image_resize_summary expects 'foo' and 'bar'.
      $image_resize_summary = array(
        '#theme' => 'image_resize_summary',
        '#foo' => $variables['foo'],
        '#bar' => $variables['bar'],
      );
      unset($variables['foo'], $variables['bar']);
      $image_resize_summary += $variables;
      return drupal_render($image_resize_summary);
    }
    
    andypost’s picture

    I'd prefere

    function theme_image_crop_summary($variables) {
      // #theme image_resize_summary expects 'foo' and 'bar'.
      $image_resize_summary = array(
        '#theme' => 'image_resize_summary',
        '#foo' => $variables['foo'],
        '#bar' => $variables['bar'],
      );
      return drupal_render($image_resize_summary);
    }
    
    thedavidmeister’s picture

    or just?

    foreach($variables as $key => $variable) {
      $variables["#$key"] = $variable;
    }
    
    thedavidmeister’s picture

    #13 is probably better than #14 and #12 as it forces us to be explicit and is more efficient.

    InternetDevels’s picture

    Assigned: Unassigned » InternetDevels

    We are working today with this issue during Code Sprint UA.

    Samvel’s picture

    Assigned: InternetDevels » Unassigned

    Thank you to both. Clear for me.

    Samvel’s picture

    @InternetDevels, sorry, not specially.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    Cottser’s picture

    Issue summary: View changes

    Remove block issue from 'fixed' section - no theme() calls there.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    I've noticed this in a few patches waiting for review:

    $foo = array('#theme' => 'foo', ... );
    $output = l(drupal_render($foo), $path, $options);
    

    In d8 literally the first thing l() does is calls drupal_render() on the $text parameter if it is an array, so it's a waste to call drupal_render() for l() like this.

    tstoeckler’s picture

    How would that be done correctly, though? That's not clear to me.

    thedavidmeister’s picture

    $foo = array('#theme' => 'foo', ... );
    $output = l(drupal_render($foo), $path, $options);
    

    becomes:

    $foo = array('#theme' => 'foo', ... );
    $output = l($foo, $path, $options);
    
    sbudker1’s picture

    Before code:

     <div  class="field field-name-field-tags field-type-taxonomy-term-reference field-label-above clearfix" data-edit-id="node/1/field_tags/und/teaser"><h3 class="field-label">Tags: </h3><ul class="links"><li class="taxonomy-term-reference-0" rel="dc:subject"><a href="/drupal/taxonomy/term/1" typeof="skos:Concept" property="rdfs:label skos:prefLabel" datatype="">ghfghh</a></li></ul></div>
    

    After code:

    <div class="field field-name-field-tags field-type-taxonomy-term-reference field-label-above clearfix" data-edit-id="node/1/field_tags/und/teaser"><h3 class="field-label">Tags: </h3><ul class="links"><li class="taxonomy-term-reference-0" rel="dc:subject"><a href="/drupal/taxonomy/term/1" typeof="skos:Concept" property="rdfs:label skos:prefLabel" datatype="">ghfghh</a></li></ul></div>
    

    Patch seemed to work! The visuals of the cite looked the same after the patch. In addition the code also looks the same!

    thedavidmeister’s picture

    #22 - looks like a cross post?

    thedavidmeister’s picture

    Issue summary: View changes

    moved history

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    jenlampton’s picture

    Issue summary: View changes

    cleanup

    jenlampton’s picture

    Issue summary: View changes

    moar cleanup

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    jenlampton’s picture

    Issue summary: View changes

    cleanup

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    jenlampton’s picture

    Issue summary: View changes

    .

    jenlampton’s picture

    Issue summary: View changes

    reorg

    jenlampton’s picture

    Issue summary: View changes

    reorg

    jenlampton’s picture

    Issue summary: View changes

    reorg

    jenlampton’s picture

    Issue summary: View changes

    reorg

    jenlampton’s picture

    Issue summary: View changes

    forum

    jenlampton’s picture

    yes, #22 was a cross post. sorry about that :)

    jenlampton’s picture

    Issue summary: View changes

    tax needs work

    jenlampton’s picture

    Issue summary: View changes

    for colors

    jenlampton’s picture

    Issue summary: View changes

    reorder

    jenlampton’s picture

    Issue summary: View changes

    unpostpone

    jenlampton’s picture

    Issue summary: View changes

    spacing

    jenlampton’s picture

    Issue summary: View changes

    up

    jenlampton’s picture

    Issue summary: View changes

    reorg

    jenlampton’s picture

    Issue summary: View changes

    reorg

    jenlampton’s picture

    Issue summary: View changes

    reorg

    jenlampton’s picture

    Issue summary: View changes

    update

    jenlampton’s picture

    Issue summary: View changes

    reorg

    jenlampton’s picture

    Issue summary: View changes

    reorg

    heddn’s picture

    heddn’s picture

    Issue summary: View changes

    Updated issue summary.

    heddn’s picture

    Issue summary: View changes

    Updated issue summary.

    heddn’s picture

    Issue summary: View changes

    Updated issue summary.

    heddn’s picture

    Issue summary: View changes

    Updated issue summary.

    heddn’s picture

    Issue summary: View changes

    Updated issue summary.

    heddn’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    jenlampton’s picture

    Issue summary: View changes

    reorg

    jenlampton’s picture

    Issue summary: View changes

    reorg

    jenlampton’s picture

    Issue summary: View changes

    reorg

    Cottser’s picture

    Issue summary: View changes

    Move up taxonomy

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    jenlampton’s picture

    Issue summary: View changes

    reorg

    jenlampton’s picture

    Issue summary: View changes

    reorg

    jenlampton’s picture

    Issue summary: View changes

    reorg

    jenlampton’s picture

    Issue summary: View changes

    reorg

    jenlampton’s picture

    Issue summary: View changes

    up

    Cottser’s picture

    Issue summary: View changes

    Reshuffle

    jenlampton’s picture

    Issue summary: View changes

    up

    Eric_A’s picture

    I've noticed conversions of low level tests around theme() and theme hook implementation output that should not be converted. These tests are a different kind of animal and need a different kind of review. Splitting off test files would speed up these issues, especially for the issues where conversion of the normal files is considered a "Novice" task. See for example #2009670: Replace theme() with drupal_render() in simpletest module and #2009674: Replace theme() with drupal_render() in system module.

    thedavidmeister’s picture

    @Eric_A, could you update the issue summary here and reorganise the remaining sub-issues as required as per #25?

    thedavidmeister’s picture

    Issue summary: View changes

    up

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    jenlampton’s picture

    Issue summary: View changes

    up

    jenlampton’s picture

    Issue summary: View changes

    up

    stevector’s picture

    Issue summary: View changes

    Fixing spelling of "temporarily"

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    jenlampton’s picture

    Issue summary: View changes

    seeing if summary needs update

    jenlampton’s picture

    Issue summary: View changes

    nr

    jenlampton’s picture

    Issue summary: View changes

    up

    jenlampton’s picture

    Issue summary: View changes

    up

    Tor Arne Thune’s picture

    Issue summary: View changes

    Cleared up a misunderstanding around the update system and the update module, as they are two different things.

    jenlampton’s picture

    Issue summary: View changes

    up

    jenlampton’s picture

    Issue summary: View changes

    up

    jenlampton’s picture

    Issue summary: View changes

    up

    jenlampton’s picture

    Issue summary: View changes

    up

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    jenlampton’s picture

    Issue summary: View changes

    up

    jenlampton’s picture

    Issue summary: View changes

    reorder

    jenlampton’s picture

    Issue summary: View changes

    p

    jenlampton’s picture

    Issue summary: View changes

    f

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    Guys, just a quick reminder that you do *not* have to try too hard to remove "early render" calls to drupal_render() in these issues. If you get some easy/obvious ones, great, otherwise don't stress it too much.

    There is already a follow-up meta that is set to explode with tasks in the near future over at #2046881: [meta] Avoid "early rendered" strings where beneficial to do so, build structured data to pass to drupal_render() once instead so don't feel like the cleanup will just disappear into the ether if we don't get it immediately here.

    We're really on the home stretch now (nearly on the last 50 calls), so if there's a choice between doing a quick conversion "1:1" or trying harder for "further cleanup", let's opt for the former and just get this out the door :)

    I'm going to do my best to keep an eye on regressions coming in from other other patches around core that have been introducing new theme() calls that didn't exist when we did our first audit.

    thedavidmeister’s picture

    Oh, and much like the Twig conversion efforts, what we need most right now are reviews.

    Right now it seems like quality feedback on patches is much harder to come by than the patches themselves.

    If you can spare half an hour to look over one of the yellow issues in the summary for coding standards violations or patches that don't follow the guidelines outlined in the issue summary, it would be super helpful to getting this wrapped up.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    jenlampton’s picture

    Issue summary: View changes

    up

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    Cottser’s picture

    Issue summary: View changes

    Update

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    jenlampton’s picture

    Issue summary: View changes

    update

    jenlampton’s picture

    Issue summary: View changes

    update

    jenlampton’s picture

    Issue summary: View changes

    up

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    jenlampton’s picture

    Issue summary: View changes

    tests

    jenlampton’s picture

    Issue summary: View changes

    up

    jenlampton’s picture

    Issue summary: View changes

    up

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    jenlampton’s picture

    Issue summary: View changes

    up

    c4rl’s picture

    Issue summary: View changes

    Update issue list

    c4rl’s picture

    Issue summary: View changes

    Fix formatting :P

    c4rl’s picture

    Issue summary: View changes

    system.module issue ready, updated summary.

    thedavidmeister’s picture

    Issue summary: View changes

    Updated issue summary.

    jessebeach’s picture

    Issue summary: View changes
    jessebeach’s picture

    Issue summary: View changes
    jessebeach’s picture

    Issue summary: View changes
    jessebeach’s picture

    Issue summary: View changes
    jessebeach’s picture

    Issue summary: View changes
    jessebeach’s picture

    Issue summary: View changes
    jessebeach’s picture

    Issue summary: View changes
    jessebeach’s picture

    Issue summary: View changes
    jessebeach’s picture

    Issue summary: View changes
    jessebeach’s picture

    Issue summary: View changes
    jessebeach’s picture

    Issue summary: View changes
    jessebeach’s picture

    Issue summary: View changes
    jessebeach’s picture

    Issue summary: View changes
    xjm’s picture

    Priority: Normal » Critical
    Issue tags: +beta blocker

    The beta blocker #2173655: Refactor theme() to _theme(); make it a private API to discourage module developers from circumventing the renderable build system is postponed on this, which makes this critical and a beta blocker as well. If it gets close to beta and this isn't done, we might want to consider doing the rename first and not blocking the beta on cleanups here.

    markcarver’s picture

    Issue summary: View changes
    joelpittet’s picture

    Issue summary: View changes
    martin107’s picture

    Issue summary: View changes

    moved install.core.inc from needs patch to needs review

    martin107’s picture

    Issue summary: View changes

    Move authorize.php up the totem pole ( from needs fix to needs review )

    catch’s picture

    Issue summary: View changes
    martin107’s picture

    Issue summary: View changes

    moving items to fixed.

    martin107’s picture

    Issue summary: View changes

    form.inc moved to fixed.

    martin107’s picture

    Issue summary: View changes

    tablesort.inc moved to review by the community

    looking at comment #1

    grep -r " theme(" . | wc -l
    358

    Count now stands at
    123

    jessebeach’s picture

    Issue summary: View changes
    martin107’s picture

    Issue summary: View changes

    Remaining - a review on 2 remaining NOVICE patches and a CRITICAL issue can be put to bed.

    martin107’s picture

    Issue summary: View changes

    All issues In the hands of the core committers now

    jessebeach’s picture

    Issue summary: View changes
    jessebeach’s picture

    We missed one #2190427: Replace theme() with drupal_render() in form.inc. I patched it and tested it on /node/add/article.

    Otherwise any remaining instances of theme() are in comments or tests. I'll address the remaining comment instances in #2173655: Refactor theme() to _theme(); make it a private API to discourage module developers from circumventing the renderable build system.

    jessebeach’s picture

    Issue summary: View changes
    martin107’s picture

    Issue summary: View changes

    core/includes moved to fixed

    webchick’s picture

    I just committed the last of these from the meta issue. WOOT!

    But unfortunately, I found a few more. :(

    ./core/modules/simpletest/lib/Drupal/simpletest/WebTestBase.php:    $output = theme($callback, $variables);
    ./core/modules/system/lib/Drupal/system/Tests/Theme/TableTest.php:    $this->content = theme('table', array('header' => $header, 'rows' => $rows, 'sticky' => TRUE));
    ./core/modules/system/lib/Drupal/system/Tests/Theme/TableTest.php:    $this->content = theme('table', array('header' => $header, 'rows' => $rows, 'attributes' => $attributes, 'caption' => $caption, 'colgroups' => $colgroups, 'sticky' => FALSE));
    ./core/modules/system/lib/Drupal/system/Tests/Theme/TableTest.php:    $this->content = theme('table', array('header' => $header, 'rows' => array(), 'empty' => t('No strings available.')));
    ./core/modules/system/lib/Drupal/system/Tests/Theme/TableTest.php:    $this->content = theme('table', array('rows' => $rows));
    ./core/modules/system/lib/Drupal/system/Tests/Theme/ThemeTest.php:   *   - $variables['attributes'] as passed in to theme()
    ./core/modules/system/lib/Drupal/system/Tests/Theme/ThemeTest.php:   * Test that theme() returns expected data types.
    ./core/modules/system/lib/Drupal/system/Tests/Theme/ThemeTest.php:    // theme_test_false is an implemented theme hook so theme() should return a
    ./core/modules/system/lib/Drupal/system/Tests/Theme/ThemeTest.php:      $output = theme('theme_test_foo', array('foo' => $example));
    ./core/modules/system/lib/Drupal/system/Tests/Theme/ThemeTest.php:    // suggestionnotimplemented is not an implemented theme hook so theme()
    ./core/modules/system/lib/Drupal/system/Tests/Theme/ThemeTest.php:    $output = theme(array('suggestionnotimplemented'));
    ./core/modules/system/lib/Drupal/system/Tests/Theme/TwigDebugMarkupTest.php:    $output = theme('node', node_view($node));
    ./core/modules/system/lib/Drupal/system/Tests/Theme/TwigDebugMarkupTest.php:    $this->assertTrue(strpos($output, "CALL: theme('node')") !== FALSE, 'Theme call information found.');
    ./core/modules/system/lib/Drupal/system/Tests/Theme/TwigDebugMarkupTest.php:    $output = theme('node', node_view($node2));
    ./core/modules/system/lib/Drupal/system/Tests/Theme/TwigDebugMarkupTest.php:    $output = theme('node', node_view($node));
    ./core/modules/system/tests/modules/theme_test/lib/Drupal/theme_test/EventSubscriber/ThemeTestSubscriber.php:      $GLOBALS['theme_test_output'] = theme('more_link', array('url' => 'user', 'title' => 'Themed output generated in a KernelEvents::REQUEST listener'));
    ./core/modules/system/tests/modules/theme_test/lib/Drupal/theme_test/ThemeTestController.php:    return theme('theme_test_template_test');
    ./core/modules/system/tests/modules/theme_test/lib/Drupal/theme_test/ThemeTestController.php:    return theme(array('theme_test__suggestion', 'theme_test'), array());
    ./core/modules/system/tests/modules/twig_theme_test/lib/Drupal/twig_theme_test/TwigThemeTestController.php:    return theme('twig_theme_test_php_variables');
    ./core/modules/views/lib/Drupal/views/Plugin/views/field/FieldPluginBase.php:    return theme($this->themeFunctions(),
    ./core/modules/views/lib/Drupal/views/Plugin/views/row/RssFields.php:    return theme($this->themeFunctions(),
    ./core/modules/views/lib/Drupal/views/Plugin/views/style/Rss.php:    $output = theme($this->themeFunctions(),
    

    It might be that the test-related ones need to stay there, but at least core/modules/views/lib/Drupal/views/Plugin/views/field/FieldPluginBase.php and core/modules/views/lib/Drupal/views/Plugin/views/style/Rss.php should be looked into more.

    There are a ton of other instances grep picks up, but they're all in comments, including out-dated ones that no longer make sense for the code below now that it's been converted to render arrays. The issue that deprecates theme() is definitely going to have its work cut out for it.

    jessebeach’s picture

    Issue summary: View changes
    jessebeach’s picture

    Status: Active » Fixed

    All sub-issus have been addressed. This critical is closed.

    jessebeach’s picture

    Issue tags: +Spark
    webchick’s picture

    Sorry, but did you see #60? I'm not sure that's true, just yet...

    thedavidmeister’s picture

    yeah, this has been happening all along. see #29

    maybe we should rename theme() to _theme() and then come back and revisit #60

    Status: Fixed » Closed (fixed)

    Automatically closed - issue fixed for 2 weeks with no activity.

    pplantinga’s picture

    I agree with #65, we should rename theme() to _theme()

    thedavidmeister’s picture

    Status: Closed (fixed) » Needs work
    jessebeach’s picture

    pplantinga’s picture

    I just looked through a grep of _theme() and couldn't find any instances that shouldn't be there. Someone else want to verify?

    catch’s picture

    Status: Needs work » Fixed

    Yes that's it's own issue, that is either fixed or has one sub-issue left.

    Status: Fixed » Closed (fixed)

    Automatically closed - issue fixed for 2 weeks with no activity.