Style Guide: Avoid toRefs with props #562
Conversation
There was a problem hiding this comment.
As far as I'm aware, this caveat of the Vue 3 reactivity system is not currently documented. I agree it should be mentioned somewhere.
I'm not sure the style guide is the right place for it. I don't see this as a 'style' issue. There are already a couple of 'non-style' recommendations in the style guide (about parent-child communication and state management) but personally I don't think they really belong in the style guide either.
There are a number of reactivity caveats with the new reactivity system but I can't find anywhere that they're currently documented. I think this would be a good fit for that section, if it existed. Writing something like that would be a significant undertaking, so it's understandable that it doesn't exist yet.
As an interim measure, I wonder whether this could be documented in the API reference for toRefs? Perhaps a general warning about missing properties, with a specific mention of props as the most common example?
| myNumber: Number | ||
| }, | ||
| setup(props) { | ||
| const { myNumber } = toRefs(props); |
There was a problem hiding this comment.
I believe the convention for documentation examples is to drop optional semi-colons.
| }, | ||
| setup(props) { | ||
| const { myNumber } = toRefs(props); | ||
| const myNumberPlusOne = computed(() => (myNumber?.value ?? 0) + 1); |
There was a problem hiding this comment.
I think the ?. and ?? operators are still too new to be used in the documentation.
To some extent this example illustrates the dangers of using ?.. It's being used to swallow an error, turning an unsubtle problem into a subtle one.
I wonder whether it's necessary for the example to show a subtle problem rather than an explosive one?
| @@ -1257,6 +1257,47 @@ While attribute values without any spaces are not required to have quotes in HTM | |||
| ``` | |||
| </div> | |||
|
|
|||
| ### Avoid toRefs with props <sup data-p="b">strongly recommended</sup> | |||
|
|
|||
| **Access props using props.propName inside `setup`.** | |||
There was a problem hiding this comment.
There's also toRef. The documentation does recommend using that for grabbing individual props but it doesn't explicitly say 'because toRefs might not work'.
| @@ -1257,6 +1257,47 @@ While attribute values without any spaces are not required to have quotes in HTM | |||
| ``` | |||
| </div> | |||
|
|
|||
| ### Avoid toRefs with props <sup data-p="b">strongly recommended</sup> | |||
There was a problem hiding this comment.
I understand the desire to single out props for special attention, as it does seem likely to be the most common example of this problem. But I wonder whether the more general case, of any reactive object with optional properties, would be worth acknowledging?
|
I've opened #639 to try to address this. |
|
@skirtles-code Oops I meant to reply but forgot! Your comments make a lot of sense and followup changes #639 look great. Thanks! |
Description of Problem
It may be tempting to standardize on destructuring props with
toRefsbut this can lead to subtle bugs.Proposed Solution
Add a style guide item to avoid
toRefswith props when possible and read props usingprops.foowhen possible.Additional Information
This is relevant when using the Composition API so this is probably an advanced issue. Feel free to ignore this or suggest alternatives. Thanks!