-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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: deep nested preact signals in props #11863
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: db7ae4d The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests are failing
859cfc9
to
24430b8
Compare
Ok, I looked at the tests and “Lit components” sometimes fails and sometimes doesn't. This has nothing to do with this change. I also jumped to the current state of the main and there it also happens from time to time. |
26aca18
to
d239fb5
Compare
Yeah I'm sure that's unrelated here. |
@ph1p would you mind resolving conflicts? |
Sure! But I'm currently on vacation. I'll do it on Saturday if it's ok :) |
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
3417f08
to
b8ec6b4
Compare
@ph1p it seems that your changes are caused some regressions in our e2e tests. Could you please look after them? |
@ph1p are you still interested in this PR? |
Yes very much @florian-lefebvre |
No problem, just wanted to check. Let us know if you need help |
Changes
This extends my work from: #11834 (review)
Now its possible to nest the signals.
If you have this:
Now the preact signal structure looks like this:
Testing
Included.
Docs
nothing should change for the user.