Skip to content
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

v4: Fix useResult type inferring 'any' #872

Merged
merged 1 commit into from
Dec 5, 2019

Conversation

bbugh
Copy link
Contributor

@bbugh bbugh commented Dec 5, 2019

useResult is great for reactive objects, but the unwrapped type is currently inferred as any. This results in unnecessary and unsafe casting. The type information is available to be inferred, but the useResult function is not using it.

This PR fixes the issue by providing stricter type inference to useResult and computed for the defaultValue and pick return values, which results in correct typing.

Issue

# Simplified for this example
interface ProjectQuery {
  project: Project
}

Usage:

const { result, loading } = useQuery<ProjectQuery>(...)
const project = useResult(result, {}, data => data.project)

❌ Typed as const project: Readonly<Ref<Readonly<any>>>

PR Fix

Usage:

const { result, loading } = useQuery<ProjectQuery>(...)
const project = useResult(result, {}, data => data.project)

✅ Typed as const project: Readonly<Ref<Readonly<Project> | Readonly<{}>>>

const project = useResult(result, undefined, data => data.project as Project) // undefined default

✅ Typed as const project: Readonly<Ref<Readonly<Project> | undefined>>

@Akryum Akryum merged commit 9edcf2f into vuejs:v4 Dec 5, 2019
@vue-bot
Copy link

vue-bot commented Dec 5, 2019

Hey @bbugh, thank you for your time and effort spent on this PR, contributions like yours help make Vue better for everyone. Cheers! 💚

@bbugh bbugh deleted the fix-useResult-typing branch December 5, 2019 19:05
@DarkLite1
Copy link

Sorry for hijacking the thread but is this PR related to fixing this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants