xstyled icon indicating copy to clipboard operation
xstyled copied to clipboard

Rename the color prop to just c

Open chrispcode opened this issue 2 years ago • 10 comments

💬 Questions and Help

Here is an example:

import React, { ComponentPropsWithoutRef } from 'react';
import styled, { x, type SystemProps } from '@xstyled/styled-components';

type ReactDiv = ComponentPropsWithoutRef<'div'>;

export interface BoxProps extends ReactDiv, SystemProps {}

function Box(props: BoxProps) {
  return (
    <BoxBase {...props} />
  );
}

const BoxBase = styled(x.div)``;

export default Box;

The code above pisses typescript because the react color prop type (string) is different than the xstyled one ( string | ThemeProp<string, Theme>).

image

If I were to omit the color prop from the react div type it would work perfectly fine

type ReactDiv = Omit<ComponentPropsWithoutRef<'div'>, 'color'>;

The question is, wouldn't it be better to rename the xstyled color prop to something else, for example just c, following a similar pattern with w and h?

chrispcode avatar May 19 '23 14:05 chrispcode

@probablyup any comments, if you don't like the idea that's fine

chrispcode avatar Jun 16 '23 12:06 chrispcode

Personally I prefer the long form of all the props because it's less confusing, albeit somewhat less convenient

quantizor avatar Jun 16 '23 12:06 quantizor

It's more about not having TS conflict than changing the prop name to something shorter. What is the suggested way of using xstyled when authoring component libraries, like the example above?

chrispcode avatar Jun 18 '23 22:06 chrispcode

I simply omit the html color property from the types, it's redundant anyway. I would rather do that than use the short form xstyled props since i think 1 letter variables are terrible

YassienW avatar Oct 09 '23 08:10 YassienW

Why do we have w, h, p, m then?

chrispcode avatar Nov 07 '23 11:11 chrispcode

Why do we have w, h, p, m then?

Legacy decision. Given that it's been around for a while, hesitant to remove, but don't want to add more of them.

quantizor avatar Nov 07 '23 11:11 quantizor

Why do we have w, h, p, m then?

Legacy decision. Given that it's been around for a while, hesitant to remove, but don't want to add more of them.

The quicker they go the less painful it will be 😄 Realistically though deprecation warnings can be added before completely removing them. Imo replacing them in a code base should be rather straight forward to automate anyway

YassienW avatar Nov 07 '23 16:11 YassienW

As far as I remember, w and h were added, so we would avoid conflicts with the image height and width attribute.

jguddas avatar Nov 07 '23 19:11 jguddas

As far as I remember, w and h were added, so we would avoid conflicts with the image height and width attribute.

How was that request different than the current one? Is it not conflicting with the native tags' color prop. If the user uses the c prop then the components expects a SystemProp type, otherwise it expects a html color prop (string).

chrispcode avatar Nov 14 '23 11:11 chrispcode

Also the request doesn't revolve around renaming the prop necessarily to c. It could be colour or hue for example. As long it does not introduce type conflicts, personally I don't mind.

chrispcode avatar Nov 14 '23 11:11 chrispcode