Deprecate several views functions

Honestly, this feels like we can just deprecate without replacement.

If you already have the view object you can already call $view->enable(), etc. The wrappers were added in 2012 in #1810788: Add enable and disable procedural wrappers which was only required for drush. I doubt these are still used by drush.

As for views_view_is_enabled and views_view_is_enabled, these were added in 2011 to use as callbacks for array_filter. In 2026 if we wanted to filter an array of view objects by their status we might use an arrow function, like array_filter($views, fn ($view) => $view->enabled()).

I'm pretty convinced we don't need it, happy to see what others think. We could also remove the test coverage for these functions, assuming that actually enabling/disabling views is tested elsewhere.

I agree with #8. Deprecate without replacement.

What do you think about:

views_set_current_view
views_get_current_view
views_embed_view
views_get_view_result

I think all these can be be moved to either Drupal\views\Views as static methods or a methods in a new service.

Agree with #14 as well, I suspect these might be used a fair bit and are not as easy to replicate by the callers.

> If you already have the view object you can already call $view->enable(), etc. The wrappers were added in 2012 in #1810788: Add enable and disable procedural wrappers which was only required for drush. I doubt these are still used by drush.

+1 to this. These methods need the view, they're about the view, and you have the view when you call them. So either add them to the View class, or use existing methods on it.

 views_embed_view

This manually checks access, but the view element also checks access in preRenderViewElement so I'm not convinced it's anything more than a wrapper around #type view at this point.

A couple comments on the MR additions.

Side note: I think it's unnecessary for views_set_current_view and
views_get_current_view (and their new replacement methods) to return by reference, since they're setter/getter functions. AFAICT they aren't used by reference in any core code, and I think it'd make sense to deprecate return by reference, but that can be for a follow up.

MR lgtm.

I also reviewed the CR, and that looks fine too.

I think we should completely deprecate Views::set/getCurrentView() as well.

The docs reference drupal_eval() which was removed in Drupal 7, and they rely on global statics. If you want the view from a route, you can use the route match. If you're doing something during rendering of a view, you should be using views hooks. When neither of those are sufficient, using those methods will likely end up with cache poisoning or similar because there's no 'current view' cache context.

Don't have a hugely strong opinion on whether that should happen here or in a followup, although if we're deprecating for Drupal 13 anyway, it might be better to do it here?

> Deprecate these and move to View

I think this should say the 'Views' class (with an s).

(Although, side note, why is that a static class and not a service?)

(Although, side note, why is that a static class and not a service?)

Per #1938030-2: Replace View's usage of drupal_container->get() with Drupal::service()

Let's introduce a Views helper object, similar to Drupal, which does what we need.

I think we should completely deprecate Views::set/getCurrentView() as well.

Checking the use of views_get_current_view() and views_set_current_view():

There is only one usage in core of views_get_current_view(), in ViewsExecutable::preExecute():

 public function preExecute($args = []) {
    $this->old_view[] = views_get_current_view();
    views_set_current_view($this);

There are only two usages of views_set_current_view, one seen above and one other in ViewsExecutable::postExecute():

public function postExecute() {
    // Unset current view so we can be properly destructed later on.
    // Return the previous value in case we're an attachment.

    if ($this->old_view) {
      $old_view = array_pop($this->old_view);
    }

    views_set_current_view($old_view ?? FALSE);
  }

The old_view property is only used in these two places. I don't think this does anything at all anymore? (Not entirely surprising since it dates back to 2009).

tl;dr: views_get_current_view and views_set_current_view can be deprecated without replacement?

The concept of the the currently executing view definitely goes beyond the route. That just covers the page display, not something like blocks or code that explicitly runs views. I think this is a valid use case.

What I'd much rather deprecate is the view properties that's set on entities, for example in \Drupal\views\Entity\Render\EntityTranslationRendererBase::preRenderByRelationship(). In core, that's used primarily by the theme suggestions/preprocess stuff such as \Drupal\views\Hook\ViewsThemeHooks::preprocessNode, which is deprecated, but the concept of that is useful and crucial for a number of features that we use in our projects (when properly considering cache implications). And if we remove that, then we'd fall back to the current view info. That falls into the set of issues about deprecating __get()/__set() on content entities.

It would probably be possibly to track this on our own using hooks, but this definitely seems easier. Especially with the non-trivial additional complexity where a view is executed within another view. A separate service could keep track of this with a push/pop thing, similar to the account switcher.

I understand there's zero test coverage for it, not going to fight it, but I'm fairly certain that removing them is going to make our use case harder in the not-too-far-future.

A few small comments on the MR.

Whoops.

1 more comment and I think it's good.

I had discussed #28/#29 a bit with @nicxvan directly.

I don't think removing and then maybe reintroducing this as a new feature in a follow-up is going to happen. Either I can convince you all to keep those functions now and convert them to a service or I'll have to figure out how work around it myself.

For context on the $entity->view properties, removing those is essentially a blocker for #3281720: [meta] Deprecate __get/__set() on ContentEntityBase, with #2896474: Provide an API to temporarily associate data with an entity as a possible 1:1 replacement for this (that issue also has a description of our use case for this), but the likely consensus is that no such API will be provided by the entity system and use cases such as views are responsible to provide their own API for it that we'll just remove them.

To be fair, there are also scenarios where this would _not_ work as a replacement for those ->view properties at all. Specifically, if we push entity rendering with a view more into (global) fibers and/or lazy builders, then it's possible that single entities are rendered outside view execution and then the concept of "current view" becomes pretty much meaningless. That's likely a good reason to remove them, as we can't promise that these functions will work as expected in the future. So I guess I just successfully disproved my own argument for keeping them ;)

Still thinking through #35.

In the meantime, had a thought that if anyone doesn't like initializing the global like this:

  $GLOBALS['_current_view'] ??= NULL;

This is good too:

  if (!array_key_exists('_current_view', $GLOBALS)) {
    $GLOBALS['_current_view'] = NULL;
  }

Yeah, I think we're good for RTBC here.

I read through @berdir's use case in #2896474-36: Provide an API to temporarily associate data with an entity, and basically, instead of creating X number of view modes for all the permutations of display options, the current view is needed, probably both in an entity_build_defaults_alter hook to customize cache keys, and then also at render time for display logic. And either the view property on the entity or views_get_current_view() makes that possible. Maybe a new issue to add an argument to EntityViewBuilderInterface::view() and ::viewMultiple() that gets passed along to the entity_build_defaults_alter hooks for additional context could address that use case?

A separate service could keep track of this with a push/pop thing, similar to the account switcher.

This sounds a lot like the request stack, but instead its a views execution stack.

I commented on the MR about using drupal_static instead of GLOBALS but that was before I read the comments here.

catch asked for GLOBALS so I can live with it.

But I'm not sure about 'there is no replacement' because I have seen some unholy things in themes with those functions.

Should we at least have a follow up for a proper replacement (eg a views execution stack)?

> Should we at least have a follow up for a proper replacement (eg a views execution stack)?

As mentioned in #35, a stack isn't really going to work in a world where we have fibers, just like the account switcher is also giving us headaches already.

Also per #35, $entity->view still exists. Unlike the global current view, it's fairly resilient against fibers and multiple views as long as you don't have a single entity being rendered by two different views on the same page. But at that point, the whole thing is pretty unpredictable anyway.

However, there's also caching to consider, which is why we deprecated the views theme suggestions as they do not work with render caching. The same is true for the current view functions and $entity->view. It's *possible* to account for that, but it's not trivial and not really documented. "I have seen some unholy things in themes with those functions." is likely exactly why we should deprecate them, unless you know exactly what you are doing, you're almost guaranteed to run into caching bugs when relying on any of that.

In most cases, when you want to vary the output by view, you should use different view modes.

Given that, as I've already convinced myself in #35, I think I'm now +1 on removing them without replacement. We likely still want to get rid of $entity->view and could possibly do a replacement of that, such as a service that you can ask to return the view that's rendering a given entity, that would also give us a place to document what steps you need to take in regards to caching if you rely on that.

If Berdir's happy I'm happy, restoring RTBC

A couple of minor review comments. Nice cleanup - the use of GLOBALS is interesting but a small price to pay for being able to remove this eventually.

@nicxvan please re-rtbc my review points were very minor and well answered...let's get this done.

Committed 188f466 and pushed to main. Thanks!
Committed 0def9dc and pushed to 11.x. Thanks!