refactor(@toss/emotion-utils): SpacingProps: Replace never with Omit for children

[SEOKKAMONI]

Overview

SpacingProps: Replace never with Omit for children

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]

👷 Deploy request for slash-libraries pending review.

Visit the deploys page to approve it

Name	Link
🔨 Latest commit	145d5fb

[okinawaa]

I want to know what effect this PR has 😊

[SEOKKAMONI]

I want to know what effect this PR has 😊

@okinawaa I should have added an explanation when I submitted the PR. I apologize.

There are various reasons, but the most compelling one is that unnecessary automatic completion of "children" in props can create confusion for developers. Instead, by completely eliminating Children using Omit, autocomplete would be removed, potentially eliminating confusion for developers.

[ssi02014]

I saw this pull request and was intrigued, so here's a small comment...! 🙏

type SpacingProps = Omit<React.HTMLProps<HTMLDivElement>, "size" | "children" | "direction"> & {
    children?: undefined;
    direction?: AxisDirection | undefined;
    size: CSSPixelValue;
}

The SpacingProps type has children?: undefined as shown above.

<Spacing direction="horizontal" size={40} />

Given the use case of Spacing, I think it's appropriate to exclude the children type, the same as @SEOKKAMONI

[SEOKKAMONI]

I saw this pull request and was intrigued, so here's a small comment...! 🙏

type SpacingProps = Omit<React.HTMLProps<HTMLDivElement>, "size" | "children" | "direction"> & {
    children?: undefined;
    direction?: AxisDirection | undefined;
    size: CSSPixelValue;
}

The SpacingProps type has children?: undefined as shown above.

<Spacing direction="horizontal" size={40} />

Given the use case of Spacing, I think it's appropriate to exclude the children type, the same as @SEOKKAMONI

I'm curious about why the type is explicitly set to 'undefined' for 'children' when it's not necessary. I think this change actually lowers the DX by providing unnecessary autocompletion to developers.

and I'm also curious why size and direction are being excluded using Omit instead of ExtendHTMLProps.

I think I might be wrong understandIf I gave me wrong understanding understandingstanding, please tell me when I got wrong understand

Lastly, thank you for always provoking my curiosity! Your comments are a great help to my learning!!

[ssi02014]

@SEOKKAMONI 👋
It seems like my intention wasn't properly conveyed! 😭

type SpacingProps = ExtendHTMLProps<
  HTMLDivElement,
  {
    children?: never;
    direction?: AxisDirection;
    size: CSSPixelValue;
  }
>;

The type of the existing SpacingProps ultimately results in the following!

type SpacingProps = Omit<React.HTMLProps<HTMLDivElement>, "size" | "children" | "direction"> & {
    children?: undefined;
    direction?: AxisDirection | undefined;
    size: CSSPixelValue;
}

You can understand this by looking at the code where ExtendHTMLProps is defined...!

export type ExtendHTMLProps<Elem extends HTMLElement, T> = Omit<HTMLProps<Elem>, keyof T> & T;

The key point is that I also agree with your opinion!! 🤗
But the maintainer's intent is important, and there may be history we don't know about 🙏

[raon0211]

Thanks! Seems that we have to exclude children from the props of Spacing.