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

Putting a ref inside a ref should unwrap it, but its possible to trick the build system so it still thinks its a ref. #10799

Open
LarsEQtiming opened this issue Apr 25, 2024 · 2 comments

Comments

@LarsEQtiming
Copy link

Vue version

3.4.19

Link to minimal reproduction

https://play.vuejs.org/#eNqNVE1z0zAQ/Ss7ujiZ8TgzwClNywSaQzgA0xa4+FBjrx0VWfJI6zQh5L+zkvNNSJtTtLvv7Vs9rVdi3DTJvEUxFCPCulEZ4c1kkfE/HA12kVSPXG5lQ+CQ2gZUpqvrVJBLBedk3RhLsALp7rCENZTW1BAxbXR1kKVlg8AFMdh/q7hOE9oyyxFusRy76e64SjXwr5SoiqEnGDmyUlc3DFt7ZG60I5BaUqbkbyzGblI3tIRr32h0wvYHWl1gKTUWN72+13eKS+aZapHRx42ZqxcRLijqx9w4aO4658ZazIkR/+F6nwQOhnQAmkk3dd+2Ol4EdocrGAzggaFQGHSgDbsxM8+Q6SWgtcYyC1MjTG8nCdxLzdP642NAP+5RuJCOYp+z6ClaVcBPZJ4NjY8n2/GMwkSZqrcZ0sOOxPeDLH8v8f5mj6FReBdDiOLuhfQuT9vvKDegMlMOz7n8Q9Ls+8apl3y+6KR/BCdWvjmyZNfpvJt8aa8oP/WwknO2Y3fne+t41wbdsvFqiZiXjNuUskqenNG8p2GWVOSmbqRC+6UhyTJSMdy+11RkSpnnTyFGtkUesovnM8x/nYk/uYWPpeKrRYd2jqnY5SizFVKXntx/5ls7SNamaBVXX0jeIb+D1mvsyj6wLSz7oC6onYavBG/1g5ssCLXbDuWF+sp1qE8Ffy8+Xhh9L/dt8i7g2GGx/gswDbUq

Steps to reproduce

Look at the reproduction link.
If we define an interface with a Ref inside and then ref an instance of that interface its unclear wether or not the inner value has been unreffed or not.
At runtime it gets unreffed, but as far as the build system is concerned the inner value is still a Ref.
This only happens if you dont initalize the value. If the value is initalized when you define the ref everything works as expected.

What is expected?

The type of field should match what it is at runtime no matter when I assign the value.

What is actually happening?

The type of field at design time is Ref<string> while at runtime its actually a string.
This only happens if the wrapping variable is assigned after it has been defined.

System Info

System:
    OS: Windows 11 10.0.22631
    CPU: (8) x64 AMD Ryzen 7 4700U with Radeon Graphics
    Memory: 799.47 MB / 23.23 GB
  Binaries:
    Node: 21.2.0 - ~\AppData\Roaming\npm\node.CMD
    Yarn: 1.22.19 - ~\AppData\Roaming\npm\yarn.CMD
    npm: 9.6.0 - ~\AppData\Roaming\npm\npm.CMD
    pnpm: 9.0.6 - C:\Program Files\nodejs\pnpm.CMD
  Browsers:
    Chrome: 123.0.6312.106
    Edge: Chromium (123.0.2420.97)
    Internet Explorer: 11.0.22621.1
  npmPackages:
    vue: ^3.4.19 => 3.4.19

Any additional comments?

This might not be a problem with Vue itself, but rather with Vite or something else in the tooling, but I'm not sure where the correct place to report the bug is. Feel free to move it or point me in the right direction if this is the wrong place for it.

@LarsEQtiming
Copy link
Author

Similar example as whats in the reproduction link:

<template>Example</template>
<script setup lang="ts">
import { type Ref, ref } from 'vue';

interface DefAsInterface {
    field: Ref<string>;
}

const initalizedAsEmpty = ref<DefAsInterface | undefined>();
initalizedAsEmpty.value = {
    field: ref('text'),
};

initalizedAsEmpty!.value.field = "changed text"; // This gives build error, but its actually valid code
initalizedAsEmpty.value.field.value = "changed text"; // This assigns to `value` which didnt exist until this line runs

const initalizedWithValue = ref<DefAsInterface | undefined>({
    field: ref('text'),
});

initalizedWithValue.value.field = "changed text"; // This gives no error, which is the correct behaviour
initalizedWithValue.value.field.value = "changed text"; // This correctly gives a build error
</script>

@john-landgrave
Copy link

john-landgrave commented May 7, 2024

I've run into this issue as well and, having dug into the reactivity types, believe that this is ultimately a conflict between the typing of Ref and the typing of the ref functions especially when you consider the documentation's recommendations for how to type ref's.

Given the section of reactivity.d.ts and ref.ts below compared to the implementation of RefImpl it is trivial to create situations where the types do not match the actual runtime values.

//  reactivity.d.ts
export interface Ref<T = any> {
    value: T;
    /**
     * Type differentiator only.
     * We need this to be in public d.ts but don't want it to show up in IDE
     * autocomplete, so we use a private Symbol instead.
     */
    [RefSymbol]: true;
}


export declare function ref<T>(value: T): Ref<UnwrapRef<T>>;
export declare function ref<T = any>(): Ref<T | undefined>;
// ref.ts

class RefImpl<T> {

// ...

  constructor(
    value: T,
    public readonly __v_isShallow: boolean,
  ) {
    this._rawValue = __v_isShallow ? value : toRaw(value)
    this._value = __v_isShallow ? value : toReactive(value)
  }

// ...

  set value(newVal) {
    const useDirectValue =
      this.__v_isShallow || isShallow(newVal) || isReadonly(newVal)
    newVal = useDirectValue ? newVal : toRaw(newVal)
    if (hasChanged(newVal, this._rawValue)) {
      this._rawValue = newVal
      this._value = useDirectValue ? newVal : toReactive(newVal)
      triggerRefValue(this, DirtyLevels.Dirty, newVal)
    }
  }
}

Using a "standard" ref:

  • If the ref is created with an initial value then the value will have toReactive() called on it
  • If assignment is done on the ref's .value, then useDirectValue will always be false and the value will have either already had toReactive() called on it at constructor time or it will get toReactive() called on it in the set value() handler.

Case 1: Typing your ref's like the docs suggest

So: the value of a ref will always have had toReactive() called on it at least once. However, if the variable is declared as the docs suggest, like below, there will be a conflict between the actual type and the type that your editor (and the TS language server) will report.

interface MyInterface {
    foo: Ref<string>;
}

const myVariable = {
    foo: ref<string>('a'),
} satisfies MyInterface;

const myVariableRef: Ref<MyInterface> = ref(myVariable);
//        ^  This will report Ref<MyInterface> where it is actually Ref<UnwrapRef<MyInterface>>

if (myVariableRef.value.foo.value) {}
// ^ This is what your editor will tell you to do even though the last .value will return undefined due to the `get` in BaseReactiveHandler

This case is a little contrived in this example because you should probably just let the type system work out the type of myVariableRef and not explicitly type it, but it is simple to extend this issue to a real case.

Case 2: Reassigning a ref
This is further complicated by the other function type declaration for ref being (I think) just wrong when compared to the actual implementation due to the fact that this overload is essentially designed for cases where the initial value of the ref should be undefined and then it should be set later.

// Considerring this:
// export declare function ref<T = any>(): Ref<T | undefined>;


interface MyInterface {
    foo: Ref<string>;
}

const myVariable = {
    foo: ref<string>('a'),
} satisfies MyInterface;

const myRef = ref<MyInterface>();

console.log(typeof myRef.value === 'undefined'); // this should log true, correctly

myRef.value = myVariable; // This is going to call `toReactive()` on `myVariable`

if (myRef.value.foo.value) {}
// Again, because the type of `myRef` is `Ref<MyInterface>` and not `Ref<UnwrapRef<MyInterface>>` your editor will think that there should be an extra `.value` on the end of this chain

The consequence of this issue is that the above code needs to be changed to something like the following to actually work at runtime and compile correctly at design time:

interface MyInterface {
    foo: Ref<string>;
}

const myVariable = {
    foo: ref<string>('a'),
} satisfies MyInterface;

const myRef = ref<UnwrapRef<MyInterface>>();

console.log(typeof myRef.value === 'undefined'); // this should log true, correctly

// One of the below options needs to be done to get the typing correct, even though none of them are required for runtime correctness -- this is a problem IMO
myRef.value = toValue(toRef(myVariable));
myRef.value = ref(myVariable).value; // This is effectively the same as above, but the `.value` on the end can get lost if the line is long
myRef.value = reactive(myVariable); // This is probably the most harmless, and is hopefully a noop considering that this is already a reactive object
myRef.value = myVariable as unknown as UnwrapRef<MyInterface>; // easily the worst option as a pattern, in my opinion

if (myRef.value.foo) {}
// The typing on this is now correct

All of this is a result of running down a bug in my work-app (so I can't share the actual code where I found this problem), but please let me know if there are any questions: I think that I have researched the problem pretty extensively.

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

No branches or pull requests

2 participants