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

Fix Solid ESLint warnings #95

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Fix Solid ESLint warnings #95

wants to merge 4 commits into from

Conversation

brw
Copy link
Member

@brw brw commented Oct 27, 2024

It's been a while since I worked on this but I figured I should get this up again.

I've been reading into Solid a bit more and it seems like you generally want to pass values instead of signals/accessor functions in order to make fine-grained reactivity possible. This is why the Solid plugin complained in the case of useControllableState. Some reading material on this:
solidjs/solid#749 (comment)
https://old.reddit.com/r/solidjs/comments/18dyxsb/why_not_pass_accessor_function_to_child_components
https://dev.to/this-is-learning/thinking-locally-with-signals-3b7h
solidjs/solid#2320 (comment)

I'm not sure if there's as much of a need for controlled components in Solid compared to in React, but if we do want to do it this way then it seems like Kobalte actually does this as well so I cheated and used their createControllableSignal as inspiration.

The more I read about Solid's reactivity the more I want to go back to React tbh but oh well lol. I feel like the Solid docs don't do a very good job of explaining why or how stuff works but maybe that's just me lacking experience with it.

@brw brw requested a review from duduBTW October 27, 2024 22:05
@brw
Copy link
Member Author

brw commented Oct 27, 2024

I thought that would request a review as soon as the PR is marked for review and not while it's a draft PR lol my bad

@brw brw removed the request for review from duduBTW October 27, 2024 22:41
@brw brw changed the title chore: fix Solid ESLint warnings Fix Solid ESLint warnings Nov 19, 2024
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 this pull request may close these issues.

1 participant