Split model values into resolved and raw

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 📌 Move form state into the global store Active 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 📌 Move form state into the global store Active

… or am I misinterpreting things now? 😅

Yes, client side - we post a model to the preview and form endpoints and there's some hard-coded stuff happening with e.g. images to flip them back from a file URL to a target ID. Look at \Drupal\experience_builder\Controller\ClientServerConversionTrait::findTargetForProps for example - that can only go away if the client-side model stores a reference to

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

As well as

[
            'src' => '/path/to/some/image.webp',
            'alt' => 'This is a random image.',
            'width' => 100,
            'height' => 100,
],

In terms of the form state, that is only a global place to store the current form values before we commit them to the model. Previously it was done via a context provider but that didn't work for ajax form additions - the global state does

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.

✨ [PP-1] Make link widget autocomplete work (for uri and uri-reference props) Postponed could also be a good example for verifying what gets implemented here.

This is blocked on 📌 Maintain a per-component set of prop expressions/sources Active which is also blocked, updating status.

Updating the title to reflect this is about default values now as 📌 Maintain a per-component set of prop expressions/sources Active includes values in the prop source data.

Once that is in, we will need default values to cover both. e.g. If we've got an image reference, the default value of the hydrated props might be a URL, alt tag and width and height, but the stored value would be the media or file ID.

📌 Maintain a per-component set of prop expressions/sources Active 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. 👍

📌 Maintain a per-component set of prop expressions/sources Active is in.

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

@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 📌 Clarify default 'resolved' vs 'source' value logic in GeneratedFieldExplicitInputUxComponentSourceBase::getClientSideInfo() Active , 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: [Front-end] 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 🐛 Default image is lost after changing props Active and 🐛 No visible error message when trying to save with missing image Active .

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 🐛 Error when adding a section to the page Active - 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?

If we can get rid of findTargetForProps then I'm open to all ideas

#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 🐛 Default image is lost after changing props Active 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...

Re #22 sorry, will focus on this today

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 ✨ Real-time preview for props changes of JS components Active . 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 📌 Support server side massage and validation of component prop form values Active 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 ✨ Real-time preview for props changes of JS components Active 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 📌 Implement endpoint for realtime preview Active will need both resolved and source. Yes, ✨ Real-time preview for props changes of JS components Active would/could/should be the first thing to actually use resolved for client-side functionality rather than sending it back to the server! 👍

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

Overhauled issue summary to reflect the names that have been chosen in 📌 Support server side massage and validation of component prop form values Active , 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 ✨ Real-time preview for props changes of JS components Active .

Annotated the MR

Two open threads, one to file a dedicated issue for the data corruption issue and another to review the use of empty

The only open thread is to open a dedicated issue, this is green and ready for review

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!

🐛 Component transforms need to be per sourceType, not per component prop Active for the critical found in the final test fail

Postponed 🐛 Component transforms need to be per sourceType, not per component prop Active on this

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

Fixed the issue spotted by @longwave

Addressed all feedback, thanks for the review

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

work here! 🤩

With this in, updated/unpostponed:

1. ✨ [PP-1] Make link widget autocomplete work (for uri and uri-reference props) Postponed
2. 🐛 Component transforms need to be per sourceType, not per component prop Active
3. 🐛 Error when adding a section to the page Active
4. 📌 Improve backend API when lots of components Active

Found a regression: 🐛 Regression caused by #3493943: example/default image in SDC no longer loads Active .

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 🙈

Automatically closed - issue fixed for 2 weeks with no activity.