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

fix(@toss/react): literal type inference #292

Merged

Conversation

ojj1123
Copy link
Contributor

@ojj1123 ojj1123 commented Jul 19, 2023

Overview

#220

I wrote a PR by modifying the type for defaultValue so that generic is not forced.

TypeScript will infer "a concrete type" if there is a generic type constraint for a primitive type.
Type constraint is applied to useStorageState so that only values that can be serialized as JSON can be received as defaultValue.

However, if defaultValue was set as a primitive type, the state was inferred to be a literal type, and generic had to be created in the following situations.

// No Generic
  const [state, setState, refresh] = useStorageState('key', {
    defaultValue: 0,
  });

  useEffect(() => {
    setState(x => x + 1);
    // ^ type error
  }, []);

// Using Generic
  const [state, setState, refresh] = useStorageState<number>('key', {
    defaultValue: 0,
  });

  useEffect(() => {
    setState(x => x + 1); // No Error
  }, []);

How To Fix

The Serializable type was inferred by dividing it into a primitive type and an object type.
Primitive types must be inferred as primitive types, not literal types (See ToPrimitive<T>), and object types are shallowly inferred, so it's okay to infer T when true for a conditional type (See ToObject<T>).

type ToPrimitive<T> = T extends string ? string : T extends number ? number : T extends boolean ? boolean : never;
type ToObject<T> = T extends unknown[] | Record<string, unknown> ? T : never;
export type Serializable<T> = T extends string | number | boolean ? ToPrimitive<T> : ToObject<T>;

p.s. Tried to solve this PR #223 as well, but state was still inferred as a literal type.

AS-IS

// Need generic
  const [state, setState, refresh] = useStorageState('@service/some-resource', {
    defaultValue: 0,
  });

  state; // 0

TO-BE

// No using generic
  const [state, setState, refresh] = useStorageState('@service/some-resource', {
    defaultValue: 0,
  });

  state; // number

PR Checklist

  • I read and included theses actions below
  1. I have read the Contributing Guide
  2. I have written documents and tests, if needed.

@netlify
Copy link

netlify bot commented Jul 19, 2023

👷 Deploy request for slash-libraries pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 4c1f8d3

Copy link
Collaborator

@raon0211 raon0211 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @ojj1123 !

@raon0211 raon0211 merged commit 858b428 into toss:main Oct 4, 2023
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

Successfully merging this pull request may close these issues.

2 participants