Skip to content

Clarify the usage of toRefs and toRef for optional props #639

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

Merged
merged 1 commit into from
Oct 24, 2020

Conversation

skirtles-code
Copy link
Contributor

Description of Problem

This was originally reported in #562.

There is a problem with using toRefs to destructure props when a prop is optional:

setup (props) {
  // This assumes `props` has a title property
  const { title } = toRefs(props)
}

The documentation seems to encourage this pattern but in practice it fails if the prop is missing as no ref will be created. This not only requires extra work to handle an undefined ref but it also causes problems if the prop is subsequently added (e.g. via v-bind="obj").

Proposed Solution

There are various ways this could be addressed through changes to the library but I've assumed that the current behaviour is correct and should be reflected in the documentation.

In composition-api-introduction.md I've just marked all the props as required. I didn't want to address the problem directly on this page so I just made this tweak to justify the use of toRefs.

composition-api-setup.md is where I've addressed the problem head-on. The current documentation describes using toRefs as 'safe', which it definitely isn't. I've added a brief explanation and an example of using toRef instead.

I've also made footnotes in the API entries for toRefs and toRef to mention this problem.

Additional Information

This is a best practices recommendation for working around a shortcoming of the library. With that in mind, I think it would be best to gather a few opinions on whether this is actually the way we want to go.

@skirtles-code skirtles-code added the discussion Topics to discuss that don't have clear action items yet label Oct 20, 2020
@NataliaTepluhina NataliaTepluhina merged commit a502ebe into vuejs:master Oct 24, 2020
nick-lai pushed a commit to nick-lai/docs-next that referenced this pull request Dec 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Topics to discuss that don't have clear action items yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants