-
-
Notifications
You must be signed in to change notification settings - Fork 6
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
Update the integration and add support signals #9
base: main
Are you sure you want to change the base?
Conversation
Wow, super interesting! What will be the difference? |
Thanks for the comments you left, I've made some changes to the PR. |
Do you add any public API for end-user? If yes, we need to test them. But I suggest that change
I also need a little explanation why we need this change? Performance? Will signals work for any modern Preact? |
core/types.ts
Outdated
@@ -0,0 +1,8 @@ | |||
import type { StoreValue } from 'nanostores'; |
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.
We use types.ts
in Nano Stores for type tests.
Can you move it to index.d.ts
?
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.
This file contains common type models for the "useStore" and "useStoreSignal" hooks
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.
Maybe it is better to put everything in single index.d.ts
and index.js
. This project is very small to use practices for huge projects.
Hi. I updated the integration to use the api provided by preact. In addition, I made an alternative integration option with support for preact signals.
There are no updated tests yet, but there was no time to understand them out.