pmndrs / valtio

💊 Valtio makes proxy-state simple for React and Vanilla
http://valtio.pmnd.rs
MIT License
8.7k stars 244 forks source link

Branded primitive types are incompatible with Snapshot<T> #747

Closed pastelmind closed 1 year ago

pastelmind commented 1 year ago

Summary

Branded/tagged types are often used to achieve nominal typing. (1, 2) Several libraries (e.g. type-fest, ts-essentials) offer utility types such as Opaque<T> to support this.

Unfortunately, the Snapshot<T> type in Valtio does not play nice with branded types. Specifically, a branded type becomes broken when passed through Snapshot<T>:

type Brand<T, B> = T & { __brand: B }
type UserId = Brand<string, 'UserId'>  // string & { __brand: 'UserId' }

const user = proxy<{ id: UserId, name: string }>({ /* ... */ })

const snapshot = useSnapshot(user)
snapshot.userId // The type is:
                //    {
                //        readonly [x: number]: string;
                //        readonly toString: (() => string) & (() => string);
                //        readonly charAt: (pos: number) => string;
                //        readonly charCodeAt: (index: number) => number;
                //        readonly concat: (...strings: string[]) => string;
                //        ... 46 more ...;
                //        readonly __brand: "UserId";
                //    }
doSomething(snapshot.userId) // Fails because the function expected a UserId
                             // but got a broken type

Link to reproduction

https://www.typescriptlang.org/play?#code/C4TwDgpgBAQgTgQwHYBMA8AVANLAfFAXigygDIoBvKAfWoCNFUAuWKAXwChRIoBVAZwhwAkikKxG6fsDgBLJAHMcAcgFDRy3By7hoauOIocoJqLJQt9ojpx099AZSQIw-ABYB7YOKcv3XtH0tAHpg0wA9AH5tUNgIABsPAHczfigAYw8wWQgxADM4DwBbJg5Yt2BgVyZQhVlgNwBXOgA6TKLgsCLUOH5ggDcEeOBZD2C6RLpggFYUAAYIdIQAdlz0gCZphBW8gEZlvLoADjmj9Yg9gBZ1y8uEAGZpvPTd9bp14P44dIHkWXj4ggWsB+NpuNAAIJIEAAMUaSHSIw8SHEAAoWhiEHAFPwWMgQABtAC6AEpCPh8WDdFAIfwAEoXQxQAAkzMGw1GDLyLBkjWgtnBUF8rk8wGECiQHjg0AIxigAB8oAARBDACByxUAWRcaHxOHxWhMiocEGAuuhhoVUAA6hAEABrbVgc0gfUWjU2u32k1mg0e2lcj0AUTghTgHoZCiDAA8wP7oXCEUikFSeML-GaMPgiCQING1ag0unReLJdK5ZFiHKWLn8xBC1AAAqFIqyQRoeH2yVJJCWysQpIIeq5TCWmtQPMFlBpDx0ABWi2AFcoUGlCBQyPiICgBIA0mYUfaICAPHliESWMWAhg90T8JwTDWgA

Check List

Please do not ask questions in issues.

Please include a minimal reproduction.

pastelmind commented 1 year ago

Any type that extends object is recursively passed through Snapshot<T>.

https://github.com/pmndrs/valtio/blob/5d0eca7edc25aa7f17fb8082ef14244a35fc12b2/src/vanilla.ts#L31-L37

All branding/tagged/opaque types I've seen use an intersection with a faux property:

// Some libraries use '__tag' or a Symbol instead of '__brand'
type Opaque<T, B> = T & { __brand: B }

This passes the T extends object check in Snapshot<T>.

Since branded types are almost always used to wrap primitives, perhaps this could be worked around by special-casing primitive types?

type Primitive = string | number | boolean | null | undefined | symbol | bigint

type Snapshot<T> = T extends SnapshotIgnore
  ? T
  : T extends Promise<unknown>
  ? Awaited<T>
  : T extends Primitive
  ? T
  : T extends object
  ? { readonly [K in keyof T]: Snapshot<T[K]> }
  : T
dai-shi commented 1 year ago

Thanks for reporting. That sounds like a good idea: https://tsplay.dev/Wobblm

pastelmind commented 1 year ago

Unfortunately that doesn't work with all type branding solutions. For one, the Opaque<T> type from type-fest (one of the most popular type utility libraries) is defined as:

declare const tag: unique symbol;                // not exported!
type Tagged<Token> = { readonly [tag]: Token };  // not exported!

export type Opaque<Type, Token> = Type & Tagged<Token>;

So directly targeting { __brand: unknown } doesn't cover all bases.

dai-shi commented 1 year ago

Do we have an ultimate solution? I thought we could only cover some typical cases. The last resort would be module augmentation, maybe.

dai-shi commented 1 year ago

I think we just let people do useSnapshot(...) as unknown as ....

pastelmind commented 1 year ago

Adding primitive types to SnapshotIgnore would solve the common case (branded primitive types). Would it help?

type PrimitiveType = string | number | boolean | null | undefined | symbol | bigint;
type SnapshotIgnore = /* everything else */ | PrimitiveType;
dai-shi commented 1 year ago

Since branded types are almost always used to wrap primitives, perhaps this could be worked around by special-casing primitive types?

Ah, I see what you mean now.

Would you like to open a PR?

pastelmind commented 1 year ago

Sure