docs(core): Update linked-signal.md #58756
Conversation
| @@ -61,15 +61,15 @@ In the example above, `selectedOption` always updates back to the first option w | |||
| ```typescript | |||
| @Component({/* ... */}) | |||
| export class ShippingMethodPicker { | |||
| shippingOptions: Signal<ShippingMethod[]> = getShippingOptions(); | |||
| shippingOptions: WritableSignal<ShippingMethod[]> = getShippingOptions(); | |||
There was a problem hiding this comment.
shippingOptions could an input, why do you think it should be describe as a writable ? It's not being written to.
There was a problem hiding this comment.
without it value is of type unknown, and so can't use .id in it
There was a problem hiding this comment.
You're probably answer to the comment I deleted, I got that. I'll investigate if we can improve that. Ideally the generics should be inferred !
There was a problem hiding this comment.
about Writable because it's not an input, now it's impossible to give it a new value
so either make it to equal input, or leave it WritableSignal IMO
I will revert this small change if you insist
There was a problem hiding this comment.
Maybe we should make it an input, wdyt @jelbourn ?
There was a problem hiding this comment.
One of my goals with this doc was is to not explicitly require people understand the component-based signal APIs first.
As to the original suggestion: there's actually nothing wrong with this being a Signal here. Any signal's value can change; just because this code in the example can't directly change it, doesn't mean the value can't change elsewhere (i.e., somewhere inside getShippingOptions there's a writable signal, but that function returns it asReadOnly)
thePunderWoman
added
action: cleanup
|
@robertIsaac Your commit message needs to be updated to fix the broken lint tests. Can you change that and force push? We can merge it after that's green. |
fix multiple issues with the existing code 1. `shippingOptions` was accessed without `this.` which produces error in realtime 2. the logic in calculating of the item exist or not was not correct since it was comparing the object to the id 3. I'm not sure why, but step 3 fail without stating typing for LinkedSignal, so I added it
|
@thePunderWoman done |
thePunderWoman
added
action: review
JeanMeche
added
action: merge
fix multiple issues with the existing code 1. `shippingOptions` was accessed without `this.` which produces error in realtime 2. the logic in calculating of the item exist or not was not correct since it was comparing the object to the id 3. I'm not sure why, but step 3 fail without stating typing for LinkedSignal, so I added it PR Close #58756
|
This PR was merged into the repository by commit e495a68. The changes were merged into the following branches: main, 19.0.x |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
fix multiple issues with the existing code sample
shippingOptionswas accessed withoutthis.which produces error in realtimeWhat is the new behavior?
fixes these issues
Does this PR introduce a breaking change?
Other information