Skip to content

HyperScript engine does not work properly with For #2453

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

Open
grimalschi opened this issue Mar 21, 2025 · 3 comments
Open

HyperScript engine does not work properly with For #2453

grimalschi opened this issue Mar 21, 2025 · 3 comments
Labels
bug Something isn't working

Comments

@grimalschi
Copy link

Describe the bug

Function "For" in HyperScript mode loses connection with signals when list changes.

Your Example Website or App

https://playground.solidjs.com/anonymous/2103f083-762b-40be-b23f-114309a0142f

Steps to Reproduce the Bug or Issue

  1. Click "add"
  2. Click "demo"

Image

Expected behavior

Here is correct behavior using TSX:
https://playground.solidjs.com/anonymous/9392c73a-cd81-4260-9f23-508104380960

Image

Screenshots or Videos

No response

Platform

  • OS: macOS
  • Browser: Vivaldi
  • Version: 7.0.3495.29

Additional context

I have also checked html`` function, it works as expected (https://playground.solidjs.com/anonymous/5e9b68a0-5d8f-4f30-8ec3-d4c8fb24f766).

The closest solution is to wrap children in function (https://playground.solidjs.com/anonymous/32865f62-3ba6-45f3-a0a1-e8bc9adced6a),
but in this case Solid redraws all children everytime I add new element in array, which is undesired behavior.

Image

@grimalschi grimalschi changed the title HyperScript engine does not properly with For HyperScript engine does not work properly with For Mar 21, 2025
@ryansolid
Copy link
Member

ryansolid commented Mar 27, 2025

Thanks yeah I can reproduce. I made some pretty heavy changes to HyperScript a couple years ago to handle Context + Fragments and while it worked then, over time we have been optimizing away unnecessary wrappers in Solid core and it turns out HyperScript is the only renderer that needs them. I want to re-evaluate this solution on a whole because basically the reactivity leaks upwards here in a way that is more or less prevented by our JSX compiler and html function.

The gist of it is that we defer evaluation here and it leads to render effects being created in the parent context instead of inside the For which means when the parent insert re-runs it inadvertently releases the render effects on the other divs. Given my current understanding of how these things should work this is all wrong.

@ryansolid ryansolid added the bug Something isn't working label Apr 30, 2025
@mizulu
Copy link

mizulu commented May 1, 2025

@titoBouzout
Copy link
Contributor

titoBouzout commented May 1, 2025

For references there are a bunch of messages and a thread about this in https://discord.com/channels/722131463138705510/780502110772658196/1367529419052285982
https://discord.gg/solidjs invite if you need

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants