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

help/Feature request: iscomputed #445

Closed
mathe42 opened this issue Jul 18, 2020 · 11 comments · Fixed by #447
Closed

help/Feature request: iscomputed #445

mathe42 opened this issue Jul 18, 2020 · 11 comments · Fixed by #447
Labels
misalignment Misalign with the latest RFC

Comments

@mathe42
Copy link
Contributor

mathe42 commented Jul 18, 2020

How can I Check if a ref is a computed ref? Need this for nuxt-composition-api...

In vue3 this is easy with @vue/Composition-API not. (const isComputed = ref => !!ref.effect) See also https://github.com/vuejs/vue-next/blob/master/packages/reactivity/src/computed.ts#L60

@pikax
Copy link
Member

pikax commented Jul 19, 2020

@vue/composition-api doesn't use effects, having that property would be confusing.

The goal of @vue/composition-api is provide an alignment with vue-next composition-api whenever possible.

The process for new features for composition api must go through vue-next or rfc.

@mathe42
Copy link
Contributor Author

mathe42 commented Jul 19, 2020

Shure but in vue3 i can check if a ref is a computed ref in @vue/composition-api not. So this is a misalignment betwen both. So this is not a new feature but a request that in @vue/composition-api we can also do this. The isComputed helper is only a method I Need and I will implement it in my code but it is not posible to write such a helper.

@jacekkarczmarczyk
Copy link
Contributor

out of curiosity - what't the use case for isComputed? @mathe42

@mathe42
Copy link
Contributor Author

mathe42 commented Jul 19, 2020

It is for nuxt-composition-api (to fix nuxt-community/composition-api#19).

We have a method useFetch and if it is used we want to hydrate the component state (so the hole return of setup is injected into the SSR generated HTML and read by the client. So useFetch is called on initial load only on server side.

If setup returns a computed ref and we can not detect that we try to set it and get an error (currently caught wit try, catch)

@antfu
Copy link
Member

antfu commented Jul 19, 2020

Hey @mathe42, as effects is not an open api, you should not rely on it. And I think, in this case, the warning is properly thrown, which means it should ask user to change their code by not passing a computed ref. I don't see any issue we are trying to solve here, I am closing it for now.

If you really think this API is useful, maybe you can open an issue or create a PR to the rfcs repo first. We would love to ship it as long as it got supported by Vue 3. Thanks.

@antfu antfu closed this as completed Jul 19, 2020
@mathe42
Copy link
Contributor Author

mathe42 commented Jul 19, 2020

effect is exposed in the types for each computed ref: https://github.com/vuejs/vue-next/blob/master/packages/reactivity/src/computed.ts#L11

So I dont think this is only internal. Or did I miss something?

@pikax
Copy link
Member

pikax commented Jul 19, 2020

https://github.com/vuejs/vue-next/blob/master/packages/reactivity/src/computed.ts#L59

  // expose effect so computed can be stopped
  effect: runner,

@mathe42
Copy link
Contributor Author

mathe42 commented Jul 19, 2020

@mathe42
Copy link
Contributor Author

mathe42 commented Jul 20, 2020

This comment is just to clarify what / why I expect some changes in @vue/composition-api = this package.

NOTE: This issue is NOT a feature request to add an isComputed helper. It is a feature request to add the ABILITY that the user can write an isComputed helper! (sorry if that was not clear... :( )

Vue 3

In the docs for Vue3 https://v3.vuejs.org/api/computed-watch-api.html#computed computed is just a ref. This is not correct for this take a look on where computed is exported / re-exported:

https://github.com/vuejs/vue-next/blob/master/packages/reactivity/src/computed.ts#L22
https://github.com/vuejs/vue-next/blob/master/packages/runtime-core/src/apiComputed.ts#L10
https://github.com/vuejs/vue-next/blob/master/packages/runtime-dom/src/index.ts#L140
https://github.com/vuejs/vue-next/blob/master/packages/vue/src/index.ts#L83

As you can see computed has the type

export function computed<T>(getter: ComputedGetter<T>): ComputedRef<T>
export function computed<T>(
  options: WritableComputedOptions<T>
): WritableComputedRef<T>

And return types:

export interface ComputedRef<T = any> extends WritableComputedRef<T> {
  readonly value: T
}

export interface WritableComputedRef<T> extends Ref<T> {
  readonly effect: ReactiveEffect<T>
}

So when I call const double = computed(() => ref.value * 2) double has a PUBLIC property effect

isComputed

So we can check in Vue3 if a ref is a computed ref const isComputedVue3 = (ref: unknown) => isRef(ref) && !!ref.effect. So it is possible to write such a handler.

@vue/composition-api

If we take a look in the code https://github.com/vuejs/composition-api/blob/master/src/apis/computed.ts#L15 computed Refs are not different than normal refs (the "readonly" we can not detect in javascript this is just in the typescript typings)

export interface ComputedRef<T = any> extends WritableComputedRef<T> {
  readonly value: T
}

export interface WritableComputedRef<T> extends Ref<T> {}

So if we have a ref we can check if it is a ref with isRef but it is not possible to create an isComputed helper - which is possible in vue3.

What does that mean?

There are multiple options

  1. Add to the Limitations that computed has not the effect property and because of that, there is no way that you can spot the differences between a ref and a computedRef.
  2. Add something like effect: null to the computed so we can decide if a ref is a computedref.
    ...

Yes there is a way to create an isComputed helper

import { isRef } from '@vue/composition-api'

export function isComputed(cmp: unknown): boolean {
  return (
    isRef(cmp) &&
    Object.getOwnPropertyDescriptors(cmp)
      .value.set!.toString()
      .includes('Computed property was assigned')
  )
}

This is a hack because checking if the code of the setter contains a string is not a good way to handle this. If @vue/composition-api changes this code breaks.

Why not an RFC

This is only a problem with @vue/composition-api and not with vue3. It's more like a hidden misalignment between both.

@pikax
Copy link
Member

pikax commented Jul 20, 2020

This is not a misalignment. vue-next and @vue/composition-api have very different implementations of reactivity, adding functionality/APIs just for the alignment is not something we strive for.

There are multiple options

  1. Add to the Limitations that computed has not the effect property and because of that, there is no way that you can spot the differences between a ref and a computedRef.
  2. Add something like effect: null to the computed so we can decide if a ref is a computedref.
    ...
  1. If a computed has both getter and setter it should behave like a ref, that's intentional!
  2. Adding a property just to be the same as the vue-next is not something we want to do, effect in a computed is an advance case and using that to detect if is a computed doesn't seem to be a good and straightforward method.

Why not an RFC

This is only a problem with @vue/composition-api and not with vue3. It's more like a hidden misalignment between both.

As explained:

@vue/composition-api doesn't use effects, having that property would be confusing.

There's no misalignment if they are not even mention on the docs.

Created #447 and still needs to go through an RFC, but if merged you can fix the original issue by checking isReadonly(computedWithoutSetter).

@mathe42
Copy link
Contributor Author

mathe42 commented Jul 20, 2020

If your PR's get approved (and merged) this would solve my problem.

Thanks for all your work @pikax & all who work on this Repro :D.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
misalignment Misalign with the latest RFC
Projects
None yet
4 participants