fix: snapshot typing for ref

[dai-shi]

fix #145

• tag ref type with const enum
• rename NonPromise to DeepResolveType (BREAKING CHANGE in types)
• use typescript instead of babel for cjs build
• use ts-jest for test

[dai-shi]

@italodeandra Please try if this fixes your issue.

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit f288379:

Sandbox	Source
React	Configuration
React Typescript	Configuration
valtio ref with React.Ref	Issue #145
valtio ref with React.Ref (workaround)	Issue #145

[italodeandra]

I'm amazed on how I didn't have to do anything for testing your change. Codesandbox bot already grabbed my cb, updated it with the commit release, and forked with it. It's working flawlessly. Thanks!

And I think it was nice that you liked my idea of the naming. Don't know how you feel about it but for me it feels nice.

[italodeandra]

Off-topic question though: Why use ts-jest for test? Isn't stock jest with babel-jest better?

[dai-shi]

Yeah, codesandbox ci is nice.

As for naming, I wasn't happy with NonPromise so your idea seems better. It's not nice to rename too often, but I don't think it's often used explicitly. I will explicitly write about the type name change in the release note.

Why use ts-jest for test?

Because babel doesn't support const enum.

[dai-shi]

@Aslemammad Do you want to review?

[Aslemammad]

Actually, it's a great type name, I don't have any problem with it, but could it be shorter?

[dai-shi]

DeepResolve? ResolveType?
DeepResolveType feels better as it looks special.

[Aslemammad]

Yea, no problem with that, let's go with it.

[italodeandra]

It's a complex name for a complex utility type 😂

And by complex I mean special like @dai-shi said.

[italodeandra]

@dai-shi I use an external typing for the proxy. But the AsRef is not export and I had to do this:

export type IMessage = {
  id: string
  content: string
  nodeRef: Ref<HTMLDivElement> & ReturnType<typeof ref>
}

Because this nodeRef doesn't trigger the AsRef from the generic type DeepResolveType, so for this case in particular it didn't work for me without adding that & ReturnType<typeof ref>.
Do you think it's possible to improve this?

Here's the CB with the problem: https://codesandbox.io/s/valtio-ref-with-reactref-problem-2-25lxs?file=/src/App.tsx

[italodeandra]

I'm also getting a strange TS error that I can't reproduce on CB:

But the workaround from my previous comment worked:

[dai-shi]

So, this doesn't work for you? (can't see it in the first screenshot.)

export const drawerState = proxy({
  currentPosition: 0,
  isOpen: false,
  isRendering: false,
  ref: ref(createRef<HTMLDivElement>()),
  startPosition: 0,
  with: 0,
})

---

We don't want to export AsRef, as it's an implementation detail and can change.
If you really need to get the ref type on your end, this is a hack:

const dummyRef =
  process.env.NODE_ENV === 'development'
    ? ref(createRef<HTMLButtonElement>())                               
    : undefined                                                               
type MyRef = NonNullable<typeof dummyRef>                                   

[italodeandra]

So, this doesn't work for you? (can't see it in the first screenshot.)

Yes, you guessed the code right. I couldn't replicate in CB, so this could be related to yarn or WebStorm. Do you have any suggestion for me to help you fix it?

We don't want to export AsRef, as it's an implementation detail and can change.

I agree with you. But in this case, I think we should aim to fix it without even needing to use this type, right?

[dai-shi]

Yeah. Oh, I missed the error code TS4023.
In this case, we'd need to export it anyway...

Or, if there were a better solution than const enum hack...

[dai-shi]

I was able to repro the error with tsc --declaration with your codesandbox downloaded locally.

Let's collect more feedbacks/ideas and see what options we have.

Meanwhile, if you need to create type files with --declaration. The workaround for now is this, which is something you already tried before this PR.

export const state = proxy({
  buttonRef: ref(createRef<HTMLButtonElement>()) as Ref<HTMLButtonElement>
});

export default function App() {
  const { buttonRef } = useSnapshot(state);

  useEffect(() => {
    console.log(buttonRef);
  }, [buttonRef]);

  return (
    <div className="App">
      <h1>Hello CodeSandbox</h1>
      <h2>Start editing to see some magic happen!</h2>
        <button ref={buttonRef as Ref<HTMLButtonElement>}></button>
    </div>
  );
}

[italodeandra]

But it feels weird to cast it like that.

createRef<HTMLButtonElement>() is exacly Ref<HTMLButtonElement>.

I didn't fix the problem, I just switched to a different one.

This solution #145 (comment) looked better to me. So I think this AsRef should be removed and I can implement the solution I mentioned on a TypeScript module augmentation. Looks better and doesn't "plaster" Valtio to a very specific use case. Because the real culprit here is how TypeScript compares two types that are the same but one is an alias and the other was deeply transpelled (I don't understand why he doesn't consider the same thing, because nothing is changed. I think it's because React's Ref is a very complex type).

[dai-shi]

Solution with module augmentation sounds nice. Can you do it with the current version?
I guess we want to change exported functions from arrow functions to function declarations.

AsRef is actually correct because we don't resolve promise for ref().

[italodeandra]

AsRef is actually correct because we don't resolve promise for ref().

Yes, you're right. I didn't think about it. But it breaks type comparisons, because every property that used ref will become PropertyValueType & AsRef and for TypeScript this makes it a completely different type from PropertyValueType. Not sure why though.

[italodeandra]

Solution with module augmentation sounds nice. Can you do it with the current version?

I edit this answer a couple times. It doesn't work with the AsRef so I had to roll back to 1.0.3.

export { ref, snapshot, getVersion, proxy, subscribe } from "valtio"

declare module "valtio" {
  type Options = {
    sync?: boolean
  }

  export declare type DeepResolveType<T> = T extends Function
    ? T
    : T extends { current: unknown }
    ? T
    : T extends Promise<infer V>
    ? V
    : T extends object
    ? {
        [K in keyof T]: DeepResolveType<T[K]>
      }
    : T

  export declare const useSnapshot: <T extends object>(
    proxyObject: T,
    options?: Options | undefined
  ) => DeepResolveType<T>
}

[dai-shi]

Oh, I didn't know you could augment const arrow function.
Do you also need to augment ref()?

I wonder if there's better way of AsRef than const enum hack.

[italodeandra]

Do you also need to augment ref()?

No, my entire augmentation is only that. But to make it work on 1.0.4 I need to remove AsRef completely on the augmentation, so I stayed on 1.0.3.

I wonder if there's better way of AsRef than const enum hack.

I'll try to think of something. This enum hack is only typing, so I can make some tests on my own augmentation and see if anything breaks on the entire code.

[dai-shi]

But to make it work on 1.0.4 I need to remove AsRef completely on the augmentation

This is probably what I meant.

This enum hack is only typing

Yes. The point of AsRef is type only. We don't want to change the JS code.

[italodeandra]

This is probably what I meant.

Oh, right. I'm currently using 1.0.3, so that's why my answer was weird. I'm currently not working on it but as soon as I have something impacted by this again I'll let you know and I'll also try to help think of something that can replace AsRef. For now let's just keep as is.