Skip to content

Style Guide: Avoid toRefs with props #562

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

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions src/style-guide/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?


**Access props using props.propName inside `setup`.**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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'.


When using the Composition API, you may consider using [`toRefs`](../guide/reactivity-fundamentals.html#destructuring-reactive-state) and destructuring `props` to have a consistent way to access reactive values.

This can lead to subtle bugs when a prop is optional. When a prop is not provided, `toRefs` will not return a key for the prop. Destructuring will give you `undefined` and the following code will not be reactive.

<div class="style-example style-example-bad">
<h4>Bad</h4>

``` js
{
props: {
myNumber: Number
},
setup(props) {
const { myNumber } = toRefs(props);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the convention for documentation examples is to drop optional semi-colons.

const myNumberPlusOne = computed(() => (myNumber?.value ?? 0) + 1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

// ...
}
}
```
</div>

<div class="style-example style-example-good">
<h4>Good</h4>

``` js
{
props: {
myNumber: Number
},
setup(props) {
const myNumberPlusOne = computed(() => (props.myNumber?.value ?? 0) + 1);
// ...
}
}
```
</div>


## Priority C Rules: Recommended <span class="hide-from-sidebar">(Minimizing Arbitrary Choices and Cognitive Overhead)</span>

Expand Down