Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Bernier154 opened this issue Aug 27, 2024 · 6 comments
Labels

Comments

@Bernier154
Copy link

Bernier154 commented Aug 27, 2024

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
Copy link
Author

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(),
            []
        );
    }
    /* [...] */
}

@statamic statamic deleted a comment from amir1387aht Aug 27, 2024
@duncanmcclean
Copy link
Member

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
Copy link
Author

Here is the quickest way to reproduce it:

  • Fresh statamic install
  • make a "debug" tag
    • image
  • Create some partials and insert the created tag in each of them
    • image
    • image
  • In the home view, use the partials and add some params to them
    • image
  • Serve and watch as the blocks get the class param from the first block
    • image

@duncanmcclean
Copy link
Member

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 }}.

@dadaxr
Copy link
Contributor

dadaxr commented Nov 29, 2024

As mentioned in #11178 , if fixing this is not so simple because of retro compatibility issues, it may be possible to add a new "self" (or another name) variable available inside partial, storing only values from that template... ( ie the same than "view" var but without the leaked values.. ) . What do you think?

@Bernier154
Copy link
Author

@dadaxr

I'm not sure because it affected the normal use aswell..

It affect repeated views especially.
In my example, I found out the bug with my "block builder" (bard field with sets) where i had an advanced parameter class.
This parameter after multiple time the same row rendered was leaking in subsequent blocks if the next block didn'nt hav a value to override it.

My solution was like you said, I scoped the bad field with "block", so i can use the "block:var" notation.

But this solution doesn't correct the behavior with view level parameters.

As for your problem, I don't think it would affect you implementation of "allExperts" (correct me) in the parent view. I don't consider data that was set above the view "leaked", I more refering to data that was set in an equal or further depth on a view :

  • mainview
    • view2
      • view 4
    • view3 (Should not have params set in view 2 or 4, unless it was a store initialized in mainview )

Your point is valid tho and there is a way to get the data from the template only:

If you check my above solution to my problem, I have rigged myself a function to get the view data only. It feels hacky because it is a private property that i forced public, but it works for me.

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(),
            []
        );
    }
    /* [...] */
}

This function only get the parameter set in the current view, so it works like the "SELF" variable proposed.

I still didn't take a look at solution for my problem because i made this temporary solution and sometime temporary become permanent untill it breaks 😅.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants
@dadaxr @JohnathonKoster @duncanmcclean @Bernier154 and others