putyourlightson / craft-sprig

A reactive Twig component framework for Craft CMS.
https://putyourlightson.com/plugins/sprig
MIT License
124 stars 9 forks source link

Sub components caching values? #140

Closed bossanova808 closed 3 years ago

bossanova808 commented 3 years ago

Describe the bug

I think I might have found a bug in Sprig. Something funny is going on with values in sub components that I can't figure out.

I have a master sprig component that loads in a bunch of matching results (sales). Each of these sales then has a sprig sub-component that is used to send emails about that sale.

On the first load of results, everything works exactly as I expect, however on the second or subsequent loads (e.g. I change a filter, so the result set of matching sales changes)...I am then seeing incorrect values in variables being passed in to the component - the one called tag in the code below. (The value that is being passed in is actually the value from the last matching result of the first load - and that value is loaded into ALL the resulting sub-components).

It feels like a caching type issue - like something is being re-used that shouldn't be. Hence I have tried printing the value 'outside' the component (which is always correct) and 'inside' the component (which on the second results and on, is not correct). I'd have thought the sub-components are wholly re-created when each result set loads in...but this legacy value is persisting for reasons I can't understand...

image

The code in question is:

                <div class="sale__section sale__quick-mailer">
                    <h3 class="bridge-accordion">Quick Email to {{ sale['Contact']|split(' ')[0] }} {{ sale['DearSaleID'] }}</h3>
                    <div class="bridge-accordion-panel" id="bridge-accordion-panel">

                        {{ sprig('bridge/_sprig_components/sales-quick-mailer', {
                            orderReference: sale['reference'],
                            email: sale['Email'],
                            contact: sale['Contact']|split(' ')[0],
                            tag: sale['DearSaleID']
                        }) }}

From that you can see that h3 value should match the value passed into the component, which is just printed in the sub-component with {{ tag }}

To reproduce

This takes quite some code to reproduce, is a mega component that loads in a bunch of results, each of which has a couple of sub-components.

Expected behaviour

I expect the sub-component to get the same value as I log immediately above the code to make the component...

Versions

I am on the latest releases of Craft and Sprig, using the provided sprig provided htmx

bossanova808 commented 3 years ago

(the value is also used in the sub component as an input...which might well be relevant...

<input type="hidden" name="tag" value="{{ tag }}">

)

bencroker commented 3 years ago

My guess is that you're coming up against complexity issues with sub-components, rather than a bug. The fact that you have an input named tag in every sub-component also suggests that simplifying your code could lead to better results. Unfortunately I can't see enough code to give you a solution, but starting with the simplest possible version and building up from there is surely your best bet.

bossanova808 commented 3 years ago

Thanks Ben (sorry, was away for a week for school holidays here!).

The fact that you have an input named tag in every sub-component also suggests that simplifying your code could lead to better results.

hmmm - each sub component needs to know the order it is dealing with (a UID from our inventory system). I am calling it tag here so as not to confuse with all the various other things labelled id. Not sure there is some other magic way for a sub component to know what it is a sub-component for, if I don't pass a value through?? My design here is in keeping with the encapsulated components philosophy, really - I am trying very much to have each component as a separate, self contained thing with explicit inputs and outputs.

Basically sprig/htmx seems to be scaling back up the DOM tree and gathering inputs well above where I'd expect it to (i.e. inputs above/outside of the surrounding form..I expected it to gather them up to that point). But this is probably my fault and I will look into it further, although the cause is non-obvious...

Am I right in thinking it should stop gathering inputs at the surrounding form tags for any sprig activated control? (I presume if no form tags, it just goes right up the tree?).

I wonder is there is some DOM structure issue in my code that I (and PHPStorm!) are missing currently that is leading to this. In any case, I can use s-vals rather than inputs, and/or filter with s-params to remove anything I really don't want being submitted, so I have a way forward. I just wasn't anticipating it climbing up the DOM like that...but when I inspected the POST there were all sorts of things from further up in there that were not intentional, so it's something in that area. Just need to work out the 'why' now.

bencroker commented 3 years ago

Am I right in thinking it should stop gathering inputs at the surrounding form tags for any sprig activated control? (I presume if no form tags, it just goes right up the tree?).

Yes, you are. Without seeing your code it is hard for me to tell what is going on and whether this might be a bug. If you can provide a stripped-down code sample (as concise as possible) that is reproducible then I can take a closer look.

There's an example that you can also reference at https://putyourlightson.com/sprig/cookbook#nesting-components

bossanova808 commented 3 years ago

Yeah it's a hard one to factor in to a simple example unfortunately.

I am still finding I need to use an s-params filter to not get these issues (and have very thoroughly checked the html to be sure there aren't structural issues).

The POST data send by the sub component is really odd and incorrect - the basic form data is all good:

-----------------------------62801852731196522413791091612
Content-Disposition: form-data; name="preview"

true
-----------------------------62801852731196522413791091612
Content-Disposition: form-data; name="orderReference"

C11111
-----------------------------62801852731196522413791091612
Content-Disposition: form-data; name="email"

jdaalder7@gmail.com
-----------------------------62801852731196522413791091612
Content-Disposition: form-data; name="name"

JemDaalder
-----------------------------62801852731196522413791091612
Content-Disposition: form-data; name="tag"

12011ef8-d5bb-4c9d-a61d-277ab2ad0f43
-----------------------------62801852731196522413791091612
Content-Disposition: form-data; name="messageType"

general
-----------------------------62801852731196522413791091612
Content-Disposition: form-data; name="trackingCompany"

-----------------------------62801852731196522413791091612
Content-Disposition: form-data; name="message"

<p>test5</p>
-----------------------------62801852731196522413791091612
Content-Disposition: form-data; name="closeCommerce"

But it gets screwy here - the variable names and values do not match:

-----------------------------62801852731196522413791091612
Content-Disposition: form-data; name="sprig:siteId"

6bfd3eb1ebed1a9c3da827fc2a478e987cfbc4c3abf97d29a55da6313b5e313c1
-----------------------------62801852731196522413791091612
Content-Disposition: form-data; name="sprig:template"

e1a39ca104931f5c08af1fc082bf1d8bf6ff17c7b8af45d0f4e4b053161bf825bridge/_sprig_components/sales-quick-mailer
-----------------------------62801852731196522413791091612
Content-Disposition: form-data; name="sprig:variables[orderReference]"

5ecace1f1f28892642969e884ff0ca692230134a109591317dad7ba44e7c2a4cC11111
-----------------------------62801852731196522413791091612
Content-Disposition: form-data; name="sprig:variables[email]"

91a4dc0b13c2e435337678a4dd2337678b4ed188189b59e8c1d0b9efac3e45f2jdaalder7@gmail.com
-----------------------------62801852731196522413791091612
Content-Disposition: form-data; name="sprig:variables[contact]"

1de3dc48e686ed425317a6fbcbed1527f1583e0e33f776c006fe8e5dc67e65faJemDaalder
-----------------------------62801852731196522413791091612
Content-Disposition: form-data; name="sprig:variables[tag]"

baa22904d8a130bb21cd5f2c92b3a837402e71db7e28bdb22cce132547d8dc4612011ef8-d5bb-4c9d-a61d-277ab2ad0f43
-----------------------------62801852731196522413791091612
Content-Disposition: form-data; name="sprig:variables[commerce]"

882bcce8f2b9d7d95e5f47f79b8b47ad2a251fc0473a7ebbf98f71b6e4741e36
-----------------------------62801852731196522413791091612--

(I have a potential file upload field in this, hence the Content-Disposition: form-data )

What is odd here is that the sprig:variables are mangled. The actual form inputs are ok and reflect the values passes in:

            <input type="hidden" name="preview" value="true">
            <input type="hidden" name="orderReference" value="{{ orderReference }}">
            <input type="hidden" name="email" value="{{ email }}">
            <input type="hidden" name="name" value="{{ contact }}">
            <input type="hidden" name="tag" value="{{ tag }}">

and looking at the POST, those values are correct

But in the ones labelled sprig:variables[tag] for example, the values are all swapped around - e.g. in that tag it is using the value for the name...

1de3dc48e686ed425317a6fbcbed1527f1583e0e33f776c006fe8e5dc67e65faJemDaalder
-----------------------------62801852731196522413791091612
Content-Disposition: form-data; name="sprig:variables[tag]"

...which really feels like a bug to me, as the creation of the component looks fine to me?

                        {{ sprig('bridge/_sprig_components/sales-quick-mailer', {
                            orderReference: sale['reference'],
                            email: sale['Email'],
                            contact: sale['Contact']|split(' ')[0],
                            tag: sale['DearSaleID'],
                            commerce: sale.CommerceStatus ?? ""
                        }) }}

...but inside the component, in the sprig:variables - those values are definitely all swapped around.

Is it possible something is going wrong with the component creation and those variables somehow...e..g I am doing something wrong in the creation (seems unlikely given the hidden inputs all have the correct values for the correct names ) - or sprig/htmx is jumbling them up in the component creation/sumission part somehow ??

(I do know from long-in-the-past experience array_merge can go spectacularly wrong, in this sort of way, if any array passed in is null - https://www.php.net/manual/en/function.array-merge.php#119355 - but no idea if that is relevant...)

bencroker commented 3 years ago

I see what you mean, but I ran a quick test locally and couldn't replicate. My recommendation is to start with a fresh sample of a sub-component and see at what stage you can replicate this behaviour.

bossanova808 commented 3 years ago

Honestly can not work this one out despite much pulling apart of the component.

If I have s-params="not orderReference,email,contact,tag,saleNotes" on the 'parent' component, things work as expected. If I don't, I sometimes see the wrong values being submitted.

Doubtless you are right and it is something at my end, but I can't work out quite where the issue is and have a workaround, so it's just noise at this point really.

Going to close until/if I can come back with something reproducible.

bencroker commented 3 years ago

Ok sounds good.

andrewmenich commented 3 years ago

I wanted to chime in here as I had almost the exact same issue as described by @bossanova808

my solution: My sub-component had a named hidden input, just like in OP's component. Removing that hidden input and instead using the s-val attribute within the sub-component fixed my issue.


The longer version for posterity is that my primary component was a table of users, similar to the Sprigboard demo. Each table row had a handful of sprig sub-components that you can use to interact with the user – send an email, remove from the list, schedule an appointment, etc.

On initial page load, everything would be fine, but when the primary table component refreshed (via pagination or some other method), all of the sub-components userId variables would be the same and not match the actual user's ID. Due to the hidden input (with a name attribute of userId) that was in the sub-components, a comma-separated string of all the userId values would get assembled automatically and sent along in the request during the primary component refresh. When the request finished it just used the last value in the comma separated string.

This one really tripped me up and it took a few reads through the docs and some Network tab debugging to figure out what was going on. It was helpful re-reading this section in particular: https://putyourlightson.com/plugins/sprig#component-state

bossanova808 commented 3 years ago

@andrewmenich

That was my initial though on solving this issue, too, but the problem with s-vals is that they're not actually quite the same as form inputs, right? So e.g. if your sub-component needs to submit to a controller, and needs to use those inputs, then you can't easily retrieve them in the controller? As in, s-vals makes those things available to the front end component on re-render, but not to the called controller. I may have misunderstood and stuffed something up, of course. (The controller I was calling is used in other places and I did not want to mess about with its 'API' as such).

I was thus unable to get things to successfully work without using the form inputs in the sub-component, and making sure the parent component did NOT submit those same inputs (so to speak). The whole thing felt like 'scope creep' in terms of the travel upwards approach to submitting of form data that happens with this. I was expecting only those things specifically in my sub-component to submit...but that is not what happens...which I guess is what #4 is describing in https://putyourlightson.com/plugins/sprig#component-state ...but to me doesn't really gel nicely with the idea of nicely contained components.

I got it all working fine (with my fix of s-params="not orderReference,email,contact,tag,saleNotes" - but it still doesn't feel right to me.

andrewmenich commented 3 years ago

@bossanova808 Yeah, that's true. In my case, I was using a component class to do handle my PHP stuff instead of a controller.

I tend to agree with you. Components aren't as isolated as I'd like them to be, but perhaps I'm trying to fit a round peg into a square hole.

bencroker commented 3 years ago

Thanks for your input @andrewmenich and @bossanova808. You're right that nested components are not entirely isolated from parent components. The reason for this is that a lot of the htmx attributes inherit values from parent elements. Also, parent components will automatically submit any input fields that their nested components contain, since they are contained within the parent. This is why using s-val:* may be the better approach in some cases, since the value is only submitted when the element initiates the request. It's also why I say in practically every video at https://putyourlightson.com/sprig/videos that using a single component is preferable over using nested components wherever it makes sense to.

bossanova808 commented 3 years ago

And there lies the nub I guess - because unfortunately nested components make a lot of sense a lot of the time. Basically any scenario where you need to retrieve some set of results, and then operate on those results.

I have definitely tried to de-couple things wherever I can and use e.g. trigger to get self contained things to re-render when needed, but the basic problem remains that it is hard to 'get list, then operate on each element on that list' in a nice clean way if there's this climb-back-up-the-DOM thing going on.

I got there, it was just....quite confusing and unanticipated.... i.e. when you're in the mindset of 'here's this little sub-component, with these things passed into it to instantiate it' - it feels like everything within that should then be nicely isolated.

Thanks for your input and help Ben - I love Sprig and am definitely pushing the boundaries a bit with this thing, it's quite a complex component in all (...for context, it basically it deals with all our open sale orders, highlighting the issues (stock shortfall or whatever) - and gives us a 'quick mailer' to message the customer as quickly as possible (and in turn feeds into our ticketing system)...with previews etc. All powered by Spring using a tiny, tiny fraction of the code we needed for the previous version in Vue 2...

image

bencroker commented 3 years ago

Yeah I understand your confusion. Nested components can work really well when designed to be rather limited in scope, but as components get more complex it can lead to unexpected results. I've wanted to do a video series on this topic actually, just a question of time and sponsors (https://github.com/sponsors/bencroker/).

bossanova808 commented 1 year ago

Just a late follow up to this (as I ran into this issue recently again...)

I see the new attribute s-disinherit. I though this might be an alternative solve, so I added it to my spri sub component:


                        {{ sprig('bridge/_sprig_components/sales-actions',{
                            orderReference: sale['reference'],
                            email: sale['Email'],
                            contact: sale['Contact']|split(' ')[0],
                            tag: sale['DearSaleID'],
                            commerce: sale.CommerceStatus ?? "",
                            openFulfillment: sale.OpenFulfilment ?? null,
                            carrier: sale.Carrier
                        },
                        { 's-disinherit': '*'
                        }
                        ) }}

Hoping that would result in ONLY those vars going through to the sub-component...but this did not work, unfortunately, so I am still having to use this on the parent component:

s-params="not orderReference,email,contact,tag,saleNotes,carrier,commerce,openFulfillment"

In theory should the s-disinherit have worked, though? Or am I confusing what it does? Or how to use it?

I think actually I might be misunderstanding that, my perspective skewed by a want for a better solution here. But if I can use that new thing to help with this, would be good to know!

bencroker commented 1 year ago

I believe s-disinherit works backwards to what you're expecting. It prevents child elements inheriting from ancestors, not the other way around. See https://htmx.org/attributes/hx-disinherit/