-
-
Notifications
You must be signed in to change notification settings - Fork 98
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
Adjust ReadOnlySignal so we cant widen the type #587
Conversation
🦋 Changeset detectedLatest commit: 57ab3de The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
✅ Deploy Preview for preact-signals-demo ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Size Change: 0 B Total Size: 83 kB ℹ️ View Unchanged
|
8be3c70
to
703dcb4
Compare
@@ -0,0 +1,6 @@ | |||
--- | |||
"@preact/signals-core": minor |
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 might need to be a major though 😅
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.
Minor for a type correction is completely a-ok, IMO.
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.
I'll expand on the changeset then to provide our reasoning to avoid excessive issues about it
703dcb4
to
2a9febb
Compare
2a9febb
to
57ab3de
Compare
Avoid inheriting from signal in ReadonlySignal so we can't accidentally widen the type by doing
const computed: Signal<any> = useComputed(() => {})
, in widening this type we would lose thereadonly
aspect of the value property effectively making it a mutable signal.Fixes #585