statamic / cms

The core Laravel CMS Composer package
https://statamic.com
Other
4.09k stars 535 forks source link

Cascade method getViewData should only return the data from the current view and not all of it. #10703

Open Bernier154 opened 2 months ago

Bernier154 commented 2 months ago

Bug description

I am currently creating a tag which takes all the param from the view and merge them to act like the AttributesBag from blade.

For this, i was using $this->context->view as a reference to the param of the current view. But i soon realized that if i didn't put the param class in each of my block using this tag, i would get a leak from a block that used the class param.

Example:

{{ partial:layout/block_1 class="cool_class" }} 
{{ partial:layout/block_2}} // if I check $this->contect->view here, it will have the class=>cool_class param

So i went out of my way by using the "view" property from the RuntimeParser to execute the Cascade::instance()->getViewData() method.

It works, but i get the same result.

I checked the method and i feel something is wrong with it. It gets the data from all views each time, and overwrite (merge) the current's view data, which feel unituitive because it's not only the view data, it's ALL the data.

    public function getViewData($view)
    {
        $all = $this->get('views') ?? [];

        return collect($all)
            ->reverse()
            ->reduce(function ($carry, $data) {
                return $carry->merge($data);
            }, collect())
            ->merge($all[$view])
            ->all();
    }

I look through the code and it's not used a lot so maybe it's an oversight.

For the time being, i'll use Cascade::instance()->get('views) and get the array i need. ButI felt it needed to be revised.

How to reproduce

  1. Create a couple partial and use them in succession in antlers.
  2. Create a tag that dumps $this->context->get('view')
  3. Watch as the params mixes the more the parser goes.

Logs

No response

Environment

Statamic
Addons: 2
Sites: 2 (Français, Anglais)
Stache Watcher: Enabled
Static Caching: Disabled
Version: 5.23.0 PRO

Statamic Addons
codems/core: dev-master
pixelastronauts/statamic-autocomplete-address-field: dev-main

Installation

Fresh statamic/statamic site via CLI

Additional details

No response

Bernier154 commented 2 months ago

I ignored both above..

If you are interested about what i wanted to do, this is how I got the view param:

class Attributes extends Tags
{

    /**
     *  A bit hacky...
     */
    private function getViewName()
    {
        $reflectionProperty = new \ReflectionProperty(RuntimeParser::class, 'view');
        $reflectionProperty->setAccessible(true);
        return $reflectionProperty->getValue($this->parser);
    }

    private function getParamsFromCascade()
    {
        return Arr::get(
            Cascade::instance()->get('views', []),
            $this->getViewName(),
            []
        );
    }
    /* [...] */
}
duncanmcclean commented 2 months ago

For this, i was using $this->context->view as a reference to the param of the current view. But i soon realized that if i didn't put the param class in each of my block using this tag, i would get a leak from a block that used the class param.

Just to clarify, you're saying that variables from previous "replicator sets" are leaking into the cascade? Are you able to ptovide an example?

Bernier154 commented 2 months ago

Here is the quickest way to reproduce it:

duncanmcclean commented 2 months ago

Thanks - I see it now.

By the way. you don't need to build a custom tag for debugging, you can just do {{ view | dump }}.