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

Manual types with null cause issues with computed #369

Open
boar-is opened this issue Oct 13, 2024 · 10 comments
Open

Manual types with null cause issues with computed #369

boar-is opened this issue Oct 13, 2024 · 10 comments

Comments

@boar-is
Copy link

boar-is commented Oct 13, 2024

Taking an example from this page, manually providing type annotations raises a TypeScript error:

// ✅ type is inferred
const state$ = observable({
  fname: 'Annyong',
  lname: 'Bluth',
  // A child is computed
  fullName: () => {
    return `${state$.fname.get()} ${state$.lname.get()}`
  },
})

// ❌ type is manually set
// TS7022: state2$ implicitly has type any because it does not have a type annotation and is referenced directly or indirectly in its own initializer.
const state2$ = observable<{
  fname: string
  lname: string
  fullName: () => string
}>({
  fname: 'Annyong',
  lname: 'Bluth',
  // A child is computed
  fullName: () => {
    return `${state2$.fname.get()} ${state2$.lname.get()}`
  },
})

It's essential for me to provide typings manually, because I use React context so I have to export state's type for the hook.

@jmeistrich
Copy link
Contributor

jmeistrich commented Oct 14, 2024

Unfortunately that seems to be a limitation of TypeScript.

There's two ways I'm aware of to solve it.

  1. Explicitly type fullName:
  fullName: (): string => {
    return `${state2$.fname.get()} ${state2$.lname.get()}`
  },
  1. Extract the type of the inferred observable
import type { ObservableValue } from '@legendapp/state';

export type State2Type= ObservableValue<typeof state2$>;

Or does anyone know of a better way to handle this?

@boar-is
Copy link
Author

boar-is commented Oct 15, 2024

For React contexts, I let the observable to infer the type. Context types still do the job:

type UserContextValue = {
  fname: string
  lname: string
  fullName: () => string
}

const UserContext = createContext<Observable<UserContextValue> | null>(null)

function UserContextProvider({
  userFromBackend: { fname, lname },
  children,
}: { userFromBackend: { fname: string; lname: string }; children: ReactNode }) {
  const value$ = useObservable({
    fname,
    lname,
    fullName: () => `${value$.fname.get()} ${value$.lname.get()}`,
  })

  // The inferred type should match the context type
  return <UserContext.Provider value={value$}>{children}</UserContext.Provider>
}

The only downside of this approach is that removing props from the Context type won't raise any error:

type UserContextValue = {
  fname: string
  // We don't need `lname` no more
  // lname: string
  fullName: () => string
}

const UserContext = createContext<Observable<UserContextValue> | null>(null)

function UserContextProvider({
  userFromBackend: { fname, lname },
  children,
}: { userFromBackend: { fname: string; lname: string }; children: ReactNode }) {
  const value$ = useObservable({
    fname,
    // But we forgot to remove it from the observable
    lname,
    fullName: () => `${value$.fname.get()} ${value$.lname.get()}`,
  })

  // No error has been risen
  return <UserContext.Provider value={value$}>{children}</UserContext.Provider>
}

@boar-is
Copy link
Author

boar-is commented Oct 15, 2024

  1. Extract the type of the inferred observable
import type { ObservableValue } from '@legendapp/state';

export type State2Type= ObservableValue<typeof state2$>;

I didn't catch that. Could you please clarify this approach with a more complete example?

@jmeistrich
Copy link
Contributor

  1. Extract the type of the inferred observable
import type { ObservableValue } from '@legendapp/state';

export type State2Type= ObservableValue<typeof state2$>;

I didn't catch that. Could you please clarify this approach with a more complete example?

It wouldn't be useful for this case, but it's a way to extract the data type of an observable. So for example:

type UserContextValue = {
  fname: string
  lname: string
  fullName: () => string
}

const state$ = observable<UserContextValue>({...})

type StateType = ObservableValue<typeof state$>;

// StateType == UserContextValue

But in this case I think you're probably best off with 1. Explicitly type fullName. I've done a lot of that in my projects - it's a bit annoying but I haven't found a way to get around it. TypeScript has trouble figuring out all the types when it refers to itself.

@boar-is
Copy link
Author

boar-is commented Oct 15, 2024

Thank you! To summarize, here's the full example with React context:

type UserContextValue = {
  fname: string
  lname: string
  fullName: () => string
}

const UserContext = createContext<Observable<UserContextValue> | null>(null)

function UserContextProvider({
  userFromBackend: { fname, lname },
  children,
}: { userFromBackend: { fname: string; lname: string }; children: ReactNode }) {
  const value$ = useObservable<UserContextValue>({
    fname,
    lname,
    fullName: (): string => `${value$.fname.get()} ${value$.lname.get()}`,
  })

  return <UserContext.Provider value={value$}>{children}</UserContext.Provider>
}

@boar-is boar-is closed this as completed Oct 15, 2024
@jmeistrich
Copy link
Contributor

Also FYI you actually can type fullName as a string. Either way works for the observable, but typing as a function also makes it usable as a function. With it typed as a function, the observable type is this:

(property) fullName: ObservablePrimitive<string> & (() => string)

So then it would be typed to allow value$.fullName.get() or value$.fullName().

But if you make it just a string like this:

type UserContextValue = {
  fname: string
  lname: string
  fullName: string
}

It's just an observable string:

(property) fullName: ObservablePrimitive<string>

So value$.fullName() would be a TS error.

@boar-is
Copy link
Author

boar-is commented Oct 15, 2024

This makes sense! I would go with string for computed values and () => string for functions. The flexibility of this library is amazing!

@boar-is
Copy link
Author

boar-is commented Oct 15, 2024

I also found out that if the computed values may return null, it must be declared as () => string | null, not string | null. Otherwise, it causes the error:

const state$ = observable<{
  fname: string
  lname: string
  // `string | null` would cause TS2769: No overload matches this call.
  fullName: () => string | null
}>({
  fname: 'Annyong',
  lname: 'Bluth',
  // A child is computed
  fullName: (): string | null => {
    return Math.random() ? `${state$.fname.get()} ${state$.lname.get()}` : null
  },
})

This makes sense! I would go with string for computed values and () => string for functions. The flexibility of this library is amazing!

So now it is not obvious what to choose 😅. I think I will stick with the () => string style.

But I had a type like this one:

export type UserState = {
  fname: string
  lname: string
  fullName: string | null
}

And it was convenient to:

fullName: (): UserState['fullName'] => {
  return Math.random() ? `${state$.fname.get()} ${state$.lname.get()}` : null
},

And now it's not possible (with ReturnType only). No ideal solution...

@boar-is
Copy link
Author

boar-is commented Oct 15, 2024

One more finding:

There's no need to manually provide a return type if the return value is not a primitive:

const state$ = observable<{
  fname: string
  lname: string
  // Will not work with the `() => T` style!
  fullName: {
    value: string
  }
}>({
  fname: 'Annyong',
  lname: 'Bluth',
  // No return type provided, no errors
  fullName: () => {
    return {
      value: `${state$.fname.get()} ${state$.lname.get()}`,
    }
  },
})

@jmeistrich
Copy link
Contributor

Oh that's a very interesting finding! And that null issue is probably something fixable. I'll reopen this to make sure I remember to take a look at that: #369 (comment)

@jmeistrich jmeistrich reopened this Oct 15, 2024
@jmeistrich jmeistrich changed the title V3: TS7023: fullName implicitly has return type any because it does not have a return type annotation and is referenced directly or indirectly in one of its return expressions Manual types with null cause issues with computed Oct 15, 2024
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