SDC+JS Component Sources: split default values into `resolved` and `source`

Split the component model values into raw and resolved.

To make sure I don't misunderstand: you're saying that this is necessary on the client-side, i.e. in the UI's data model?

Also, AFAICT this relates to #3492511: Move form state into the global store too? Is that a third kind of value?

• raw: as stored in a FieldType plugin on the server side
• resolved: as resolved into a shape expected by an SDC prop
• form: as represented by a FieldWidget for a FieldType, aka #3492511: Move form state into the global store

… or am I misinterpreting things now? 😅

I believe this change is critical for XB in the future, because it'll make a lot of magic (see the method mentioned by Lee) explicit. Bumping priority.

#3499279: Make link widget autocomplete work (for uri and uri-reference props) could also be a good example for verifying what gets implemented here.

#3493941: Maintain a per-component set of prop expressions/sources is no longer postponed.

Currently reviewing #3493941, and now #7's second paragraph makes sense to me: that part of this MR's scope was merged into #3493941. 👍

#3493941: Maintain a per-component set of prop expressions/sources is in.

We're threading very closely to this over in the MR for #3501902: Adding the Image component results in a state considered invalid.

@larowlan is out this week 🏝️

The need for this was confirmed by @larowlan at #3501902-67: Adding the Image component results in a state considered invalid.

I just created #3507749: Clarify default 'resolved' vs 'source' value logic in GeneratedFieldExplicitInputUxComponentSourceBase::getClientSideInfo(), which I believe will make this issue much simpler, because with that done, the server-side pieces will already exist.

We ran into the actual real-world need for this: #3507929-25: Allow adding "image" props in the code component editor.

This blocks #3487284. See #3487284-12.

Bumping priority given this is in the critical path.

I bet this will also help solve #3508077: Default image for SDC with optional image is lost after changing value for another SDC prop and #3509608: Unable to save an optional image with its default value (because it does not correspond to a Media entity): allow saving DefaultRelativeUrlPropSource.

Been thinking about this on and off for a while now. I agree that we need the raw values available to the client side UI, because in the case of

[
            'alt' => 'This is a random image.',
            'width' => 100,
            'height' => 100,
            'target_id' => (int) $this->mediaEntity->id(),
]

we need to send and receive the target_id for the media library form. This is the root cause of #3511451: Cannot publish image or save as section in a particular component - see Felix's investigation in #7.

Do we only need resolved values for component preview and final render? I have concerns that if we are storing/sending both raw and resolved then there is a chance of them getting out of sync - either accidentally, or deliberately if someone is trying to mess with the values, which might have security implications.

So in the layout API we would only send the layout + raw values to the client, the client stores and sends raw values back for forms (which is all we need anyway) and previews (which we translate at preview time into the resolved values). If this is correct, where do we need resolved values in the client side model?

#21: right, but as you already wrote in #4 >3 months ago and again confirmed in #7, that's exactly what this issue will make possible.

Let's get this done ASAP, this would've prevented me having spent hours on #3508077: Default image for SDC with optional image is lost after changing value for another SDC prop today. 😬

As described in #3508077-28: Default image for SDC with optional image is lost after changing value for another SDC prop: there's a front-end bug lurking that causes some props' sources to not be passed from the client to the server, even though they should be set to the default as returned by /xb/api/config/component: https://git.drupalcode.org/project/experience_builder/-/merge_requests/7...

Do we only need resolved values for component preview and final render? I have concerns that if we are storing/sending both raw and resolved then there is a chance of them getting out of sync

I agree. Ideally only the raw values would need to be passed from the client to the server. As to where within the React code the resolved values are needed at all, as well as whether or not it's needed for resolved values (in addition to raw values) to be passed from server to client, I opened #3514910: Refinements of real-time preview for props changes of JS components. That issue should be relatively straightforward to do, and I wonder if it would help with this issue's implementation to do that one first, to make sure resolved values are being used somewhere within the React code, even if not as part of the HTTP API.

First: please check #3499550: Support server-side massaging and validating of ComponentInputsForm values for why @larowlan introduced source in addition to the pre-existing resolved. @longwave in #20's comment seems to suggest XB stores both, but that's not the case! We only store the source in the DB — the resolved really just dates back to the super simple client-side data model we had until before that issue.

I don't see how #3514910: Refinements of real-time preview for props changes of JS components would A) help this issue, B) be relatively straightforward?

Yes, the resolved value is currently not yet actively used on the client-side, and we could in theory remove it. But there's little value in doing that, because #3453690: [META] Real-time preview: supporting back-end infrastructure will need both resolved and source. Yes, #3514910: Refinements of real-time preview for props changes of JS components would/could/should be the first thing to actually use resolved for client-side functionality rather than sending it back to the server! 👍

But #3514910: Refinements of real-time preview for props changes of JS components kinda can't be straightforward — you already mentioned the auto-save implications, I just added another: undo/redo → #3514910-2: Refinements of real-time preview for props changes of JS components.

Overhauled issue summary to reflect the names that have been chosen in #3499550: Support server-side massaging and validating of ComponentInputsForm values, and what the remaining work here is.

I don't see how #3514910: Real-time preview for props changes of JS components would A) help this issue...Yes, the resolved value is currently not yet actively used on the client-side, and we could in theory remove it.

My thinking was that the help it would offer this issue is by having an implemented use case (with tests) of the resolved values being used. So that, a) it prevents them from being removed, and b) it allows refactoring of where they're kept if such refactoring is desired per #20 while continuing to ensure that such refactoring is compatible with the use-case they need to serve.

But I'm totally okay with this issue being worked on first and not postponing it on #3514910: Refinements of real-time preview for props changes of JS components.

One nit with test formatting but otherwise no notes, this looks great - good find on that stored data vs later shape change bug, thanks also for the detailed self-review.

🥳 Reviewing!

Feedback addressed, added another followup as per discussion, back to RTBC.

Waiting on this to land before I try to move the other image issues forward because this is intertwined with some of them.

Found a bug when testing this with xb-demo, see MR.

However the server side performance issues with xb-demo are just gone with this patch, the 70 calls to GeneratedFieldExplicitInputUxComponentSourceBase::getClientSideInfo() now take 92ms down from 21,158ms - a 230x speed improvement :D

All points addressed, no further feedback - over to Wim for hopefully the final review.

We fixed a complex set of problems by net-deleting code: MR diffstat is +208, −325 🥳😄

Plus, now section 3.2.1 Source vs resolved input in docs/redux-integrated-field-widgets.md is actually completely accurate and doesn't even need to spell out that the same is true for the default values — that just makes sense, it's the situation before this MR that didn't make sense! (Well, just was unfinished/inconsistent.)

Phenomenal work here! 🤩

With this in, updated/unpostponed:

1. #3499279: Make link widget autocomplete work (for uri and uri-reference props)
2. #3515629: FieldWidget's XB transforms must be bubbled by the Field Widget rendering to inform the client
3. #3511451: Cannot publish image or save as section in a particular component
4. #3515510: Improve backend API performance: add caching to shape matching

Found a regression: #3517102: Regression caused by #3493943: existing optional image SDCS with example/default no longer render, adding SDCs with optional image to canvas fails.

The regression I reported in #46 makes it so that I shouldn't tag another release until that's fixed; it's likely too disruptive.

I should've done manual testing, if I'd done that, I'd have caught this. I was too eager to land this 🙈