-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
feat: When closing over values in server$
the values should be marked as readonly
#5238
Conversation
…should be marked as readonly Co-authored-by: wajihaNiazi <waijehaniazi204@gmail.com> Co-authored-by: Phu Nguyen <51897872+phunguyenmurcul@users.noreply.github.com>
👷 Deploy request for qwik-insights pending review.Visit the deploys page to approve it
|
Love this change. Let me know when it is ready. |
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.
Can you also add an e2e spec that tries to change an object on the server? Something like
server$(() => {
try {
myObject.deeper.foo = 'test'
return false
} catch (e) {
if (/* some test that checks it errored because of freeze */)
return true
return false
}
})
Hi @gitstart thanks for your help, are you still on this PR or is it abandoned? |
Sorry for the delay. we are still working on the issue |
…should be marked as readonly Co-authored-by: Hameed Abdulrahaman <hameedabdulrahamann@gmail.com> Co-authored-by: wajihaNiazi <waijehaniazi204@gmail.com> Co-authored-by: Phu Nguyen <51897872+phunguyenmurcul@users.noreply.github.com>
@gitstart could you rebase? |
@gitstart still interested in finishing this? |
server$
the values should be marked as readonlyserver$
the values should be marked as readonly
in the future technically we can pass the current client value to the server then update the client signals from the server response |
What does this PR do?
This PR ensures that values serialized by server$() are immutable and triggers a clear error message if any mutation is attempted
Issue reference
#5019
Demo video/scrennshot:
https://www.loom.com/share/abc8b587001d4845bc55748000b0412f?sid=945d461e-79cc-4cd3-a80f-0e99bb0aa065
Happy hacking! 🎉
This code was written and reviewed by GitStart Community. Growing great engineers, one PR at a time.