- Issue created by @effulgentsia
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
- Assuming we can get the updated prop value client-side, we can update the preview by simply targeting the
<astro-island>
element for this component instance, and updating itsprops
attribute. The Astro island will then automatically re-render itself client-side based on the new props.
I forget the details of how our undo/redo functionality is implemented exactly (and I see we're missing docs for it other than a one-liner 🫣), but … AFAICT this would then have to add Component Source-specific logic to perform undo+redo. It'd need to check every affected component instance when undoing/redoing, check if it's provided by the
js
ComponentSource
plugin, and then perform this alternative.
And if and only if there's zero other component instances affected, then that's all that's needed. But if there's >=1 component instance from another component source that needs undoing/redoing, we still need to talk to the server, because the server can only update the entire preview, not a subset.(And I bet there's more complications.)
- Assuming we can get the updated prop value client-side, we can update the preview by simply targeting the
- 🇺🇸United States effulgentsia
Is there a reason we don't include the preview HTML in the undo stack? Thereby allowing all undo operations (regardless of what kinds of components are on the page) to optimistically render before the server response?
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
Fair question. I suspect because "memory usage will go through the roof".
- Assigned to jessebaker
- 🇺🇸United States effulgentsia
Tagging this as a beta blocker, because we want early adopters of the beta able to run XB in production, including under potentially heavy server load.
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
I think @effulgentsia implies with "heavy server load" that he wants Content Creators using XB to have a good experience, which implies a low-latency experience, even when latency is high (either high client → server network latency or simply the server response latency being high, aka high server load).
If so, can we start with implementing this while accepting that not every prop's resolved value (see:
EvaluatedComponentModel
My concrete proposal:
- for many prop shapes, we already have client-side transforms for the used field widget
- we have JSON Schema information for each such prop, which allows client-side validation of the resolved value against the JSON Schema
- restrict scope of this issue to only those prop shapes where #1 (a client-side transforms) exists, and which meets #2 (transforms to a valid resolved value per the JSON Schema for that prop)
- leave EVERYTHING ELSE to follow-up issues: A) client-side transform exists but does not pass client-side JSON Schema validation, B) client-side transform does not yet exist but is possible, C) client-side transform to resolved value is impossible (for example: media library widget), but we could do some client-side caching — A+B+C can then be follow-ups that improve the state this issue would put us in.
That way, we can start implementation any time (even today), and learn what the most valuable missing pieces would be.
- 🇫🇮Finland lauriii Finland
restrict scope of this issue to only those prop shapes where #1 (a client-side transforms) exists, and which meets #2 (transforms to a valid resolved value per the JSON Schema for that prop)
Where can I find a list of prop types that meet this criteria?
- 🇺🇸United States effulgentsia
Postponing this on ✨ Unwrap astro-islands and astro-slots Active , because that issue will make this one more complicated, because once we remove the astro wrappers, it's less clear how to then target the component instance and re-render it with updated props. It's definitely solvable, just trickier.
Related, it doesn't strictly need to block a beta despite #5. Though I still stand by #5 as a strong nice-to-have, so instead of removing the "beta blocker" tag entirely, this is now our first issue that I'm tagging as a "beta target". And now that we have our first one, we'll likely start using this tag for some other issues as well :)
- 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
Starting on this on top of ✨ Unwrap astro-islands and astro-slots Active which has an MR now.
- 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
we can update the preview by simply targeting the element for this component instance, and updating its props attribute
In my testing (Firefox) this updates the HTML inside the iframe's
contentDocument
but doesn't trigger re-rendering.I wonder if that's because we're making use of the
srcdoc
attribute - 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
I wonder if that's because we're making use of the srcdoc attribute
Ah I think this might be that I'm updating the wrong iframe, I forget we did the swapping
- 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
Ah I think this might be that I'm updating the wrong iframe, I forget we did the swapping
No joy, I'm definitely updating the write iframe/component
The screenshot shows I've changed the text prop to the value
'hotdog'
but as you can see the<p>
still contains the original value 'Some text'I've confirmed that the attributeChangedCallback is calling
hydrate
and that is seeing the new prop value. - 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
Ah, the preact client bails early if there's no ssr attribute and the first hydrate call removes it
And if we
setAttribute('ssr')
that triggers a hydrate call which then removes itself again. So I think we might need something different to<astro-island>
like we did in ✨ Unwrap astro-islands and astro-slots Active but that ignores the ssr attribute.Seeing what other options we have.
- 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
Ok, got something along those lines going with this code in the console and a custom element
<xb-island>
ai = Array.from(document.querySelectorAll('iframe')).reduce((c, i) => c.concat(...Array.from(i.contentDocument.querySelectorAll('xb-island'))), []).forEach(e => {delete (e.__k);e.setAttribute('props', JSON.stringify({...JSON.parse(e.getAttribute('props')), text: ['raw', 'hotdog']}))})
the
delete (e.__k)
is to remove the node preact adds for diffing. With that present it always tries to update existing elements but they don't exist because the innerHTML has been blown away by the hydration processWhere
<xb-island>
isconst AstroIsland = customElements.get("astro-island"); class XbIsland extends AstroIsland { constructor() { super(); this.addEventListener("astro:hydrate", () => { // Add the ssr attribute back so we can re-hydrate when props change. this.setAttribute('ssr', ''); }); } } customElements.define("xb-island", XbIsland);
Will do some more digging to see if I can find a way to make use of the existing __children node preact retains
- 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
Current train of thought on this
* When we're in preview, have the unwrapped-astro-island add a template element with its props and children on it during unwrap. Place another template element after the last child so we have markers for where the replaced (unwrapped) HTML goes to and from.
* In XB UI emit a custom event on the active iframe preview in the inputBehaviorsComponent input form when a component prop changes _and_ it has passed validation _and_ we know it doesn't need a server side trip (can probably assume this will be fine for any scalar prop. If the event from that
* When we're in preview have unwrapped-astro-island and xb-island listen for these events and update their markup - in the case of xb-island by something similar to #14 but hopefully without needing to know the internals of preact. In the case of the unwrapped island by doing something with the stashed template elementsWill continue on monday
- 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
Now that ✨ Unwrap astro-islands and astro-slots Active is postponed, we don't need to postpone this on that.
Got a working version of this in the branch
- 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
This is green and ready for review w/ test coverage
- 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
larowlan → changed the visibility of the branch 3514910-pp-1-real-time-preview to hidden.