Ignore morphing children in same cycle when ignore-morph is added to parent#1124
Ignore morphing children in same cycle when ignore-morph is added to parent#1124wmTJc9IK0Q wants to merge 1 commit intostarfederation:developfrom
Conversation
|
I did not open an issue first because I already had the code but this can be treated as a discussion. |
There was a problem hiding this comment.
Pull request overview
This PR enhances the data-ignore-morph attribute to work in the same patch cycle when it's added, addressing a performance optimization use case for elements backed by expensive queries.
Changes:
- Added early-return logic in
morphNodeto detect when a new element hasdata-ignore-morphattribute and apply it to the old element without morphing children or attributes
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Nice to see this. It seems quite similar to the issues I raised in this Discord bug report/thread, but the discussion there was not fruitful I fixed it to my liking in this commit, but not sure if our issues were sufficiently similar, or which solution (or combination thereof) is most appropriate. Perhaps you could create a codepen that replicates the issue that you are currently facing, so that we can more easily understand your issue and compare our solutions? Here's one for my issue: https://codepen.io/nickchomey/pen/NPrpXmw |
|
In your example you are having to track state between frames anyway. So just do that, render it once with content and then once without. The problem with your patch, is it forces all uses of |
|
@andersmurphy who/which comment/issue are you replying to? |
Why should this be the default? And, to @andersmurphy’s point, how would users morph in elements that should be morphed once, but not in future? |
What if the current update doesn’t know what the next update will be? In my example, I don’t know if the next update will be from a user interaction where I don’t want rerun the query or if it’s from a refresh of the query pushed from the backend where I do want the element to be updated.
Not sure I am following. In my backend code with this version I only have to track whether or not I want the query to be executed on the current render. It seems easier to follow that the current update knows whether or not this update should morph a specific element rather than the previous update.
I don’t know if it should be. I guess I don’t understand the intended usage of current behavior and why it’s different than data-preserve-attr. Maybe a better solution is a new data-preserve-children. I think maybe this can be implemented in a plugin now that morphing is done via a watcher plugin instead of in the engine? |
|
I think the main question of this thread should be how to solve the problem statement. If it can be done well without changes to Datastar, great. I haven’t found another way I can meet all of these requirements:
Is there a pattern that can do this without some way to mark an element as “don’t morph this because I didn’t refetch it on this update”? |
|
@nickchomey I haven’t yet looked at your issue but I’ll do it soon. If a codepen is necessary to understand the latest requirements I wrote, let me know |
Thinking about this and @andersmurphy’s comment some more. IIRC my proposed change here only affects updated elements but not new elements. New elements still get created (including their children) when they have ignore-morph set. So the case you all talking about is where an element is updated (but not created) with ignore-morph set and then any future updates to that element are ignored, including that element not being included in its parent in future updates to the parent? If that’s right, then yes this change would not be compatible with that pattern and a new preserve-children would be best. But I’m not sure if I’m understanding this right. |
The moment you say that frontend state is independent of backend state across updates, you’re managing state in two places. The recommended approach to handling this is using signals to inform the backend of frontend changes, which can then be accounted for in the next patch elements event. This is how I do it in an app where the user can add multiple filters. Instead of each filter consisting of several signals (resulting in tens or hundreds of signals), I combine all filter values into a single signal that the backend receives. The alternative, which I don’t think is a good pattern, is to add |
I confirm that I have read the contribution guidelines and will include a clear explanation of the problem, solution, and alternatives for any proposed change in behavior.
Problem Statement
I have a page of components that are backed by slowish queries. I am using the single-SSE connection approach for re-rendering the whole page and relying on morph to make any necessary changes to the DOM.
I do not want these slow queries to run on every UI interaction that needs to re-render the page. Instead, I want to preserve the current state from the last time these queries were executed but morph everything else that changed.
This worked fine when the thing I was rendering was a web component, and I was using
data-preserve-attr:my-chart-data. If I added this attribute, datastar's morph wouldn't modify themy-chart-dataattribute on that patch/merge pass, and then the web component's observed attributes wouldn't change so the render wouldn't get recalled.However, I wanted to do the same for a regular HTML element, and avoid morphing any of the children of an element backed by one of these slow queries. I found that
data-ignore-morphdoesn't behave the same way and doesn't apply until the NEXT morph pass.Proposed Solution
Check if the new version of the element has
data-ignore-morphand if it does, only apply that change to the element and short circuit updating anything else, including children.I have been running with this in prod for months now with no problems.
Alternatives Considered
I tried to find any way to get this to work without this change and I could not. I am also following the "best practice" CQRS pattern here.
I want a simple render path but I also want to avoid re-running expensive queries to make simple UI updates.
Let me know if there's another way I am missing.