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

classes defined statically in classList are missing after reactive overriding #2341

Open
MrFoxPro opened this issue Oct 18, 2024 · 2 comments
Labels
enhancement New feature or request
Milestone

Comments

@MrFoxPro
Copy link
Contributor

MrFoxPro commented Oct 18, 2024

Describe the bug

classList={{
  "b-1px b-solid b-red": true,
  "b-1px b-solid b-blue": <reactive value>,
}}

After changing reactive value,b-1px and b-solid tokens will disappear.
This is not critical, because of how CSS works, this kind of expression probably is user's mistake, but behaviour is unexpected. We should better handle this case.

Your Example Website or App

https://playground.solidjs.com/anonymous/ad8fd905-fb6e-45cb-8bdc-95ad0dd309f7

classList probably should not use node.classList.toggle directly, or compare individual tokens

@MrFoxPro MrFoxPro changed the title classList probably should not use node.classList.toggle classes defined statically in classList are missing after reactive overriding Oct 18, 2024
@MrFoxPro
Copy link
Contributor Author

I belive it will affect performance, so maybe we can have some kind config flag to enable more consistent approach?

@ryansolid
Copy link
Member

I see.. Not using toggle sort of defeats the point of this optimization and diffing it is tricky. I think you are right though that this is unexpected behavior. I doubt we'd change anything here in 1.0 lifespan. But this will definitely weigh in on how we want to handle classList moving forward.

Thanks for reporting.

@ryansolid ryansolid added the enhancement New feature or request label Oct 21, 2024
@ryansolid ryansolid added this to the 2.0.0 milestone Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants