Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot reuse a dom element when switching mount locations #2357

Open
thetarnav opened this issue Nov 9, 2024 · 6 comments · May be fixed by ryansolid/dom-expressions#382
Open

Cannot reuse a dom element when switching mount locations #2357

thetarnav opened this issue Nov 9, 2024 · 6 comments · May be fixed by ryansolid/dom-expressions#382

Comments

@thetarnav
Copy link
Contributor

thetarnav commented Nov 9, 2024

I'm trying to reuse the same dom element, and attach it in different places in the ui.
But even though that element is rendered only in one place at a time, solid's rendering of it is not stable.
Sometimes the element is added correctly, sometimes it's removed from the dom completely and sometimes it appears in the wrong spot.

const my_dom_element = document.createElement('div')

return <>
	<div>
		<Show when={something() === 'foo'}>
			{my_dom_element}
		</Show>
	</div>
	<div>
		<Show when={something() === 'bar'}>
			{my_dom_element}
		</Show>
	</div>	
</>

Playground link:
https://playground.solidjs.com/anonymous/d2ce885b-d473-4641-94a6-d9c659bc1e22

I know that this can be "solved" by creating new elements and animating them as if they were the old one. But that is besides the point.
This just seems like something that should work.

@titoBouzout
Copy link
Contributor

Smaller repro https://playground.solidjs.com/anonymous/e8cab030-93c5-44b5-8f26-f283547cc77e

We have talked about this with Fabio in the past, the conclusion IIRC was that before removing the element we should check if the parent is still the same.

@titoBouzout
Copy link
Contributor

It still breaks on this scenario, so I suppose that also won't help
https://playground.solidjs.com/anonymous/7c9fcf09-82a0-4131-a1ba-51a37d50dca0

@thetarnav
Copy link
Contributor Author

I think it would be fine even if you had to explicitly tell the renderer that you want to reuse some element.

const el = createReusable(() => <div/>)

This is a rare case after all

@mdynnl
Copy link
Contributor

mdynnl commented Nov 11, 2024

It seems this is a duplicate of #2030 with almost the same repro as #2357 (comment)

Edit: nah, only the last comment #2030 (comment)

@titoBouzout
Copy link
Contributor

titoBouzout commented Nov 11, 2024

There's also ryansolid/dom-expressions#337

And cross posting Voby vobyjs/voby#39 for this very same issue

@mdynnl
Copy link
Contributor

mdynnl commented Nov 11, 2024

https://discord.com/channels/722131463138705510/1217034387938607145

Yes, but as soon as you put the components in a seperate div, it breaks.

This is exactly what's happening here. And the workaround (for now) is to simply group the nodes https://playground.solidjs.com/anonymous/dfea14d8-e6db-4b76-a3f2-12b3ce0335db

And here's a repro with (almost?) all the ways this can break.

We can solve this by tracking whether a node was moved then ignoring the removal of tracked nodes. (PR coming up)

And then in the case of multiple insertions, intuitively (and how it works currently), the last insertion wins and as a consequence, the internal rendering state is inconsistent with the actual DOM state. Although it's not breaking anything, there's the question whether we should keep this behavior or somehow keep the internal state consistent or keep only the first insertion.

@mdynnl mdynnl linked a pull request Nov 11, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants