styled-components / xstyled

A utility-first CSS-in-JS framework built for React. 💅👩‍🎤⚡️
https://xstyled.dev
MIT License
2.28k stars 105 forks source link

Rename the color prop to just c #399

Open chrispcode opened 1 year ago

chrispcode commented 1 year ago

💬 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 commented 1 year ago

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

quantizor commented 1 year ago

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

chrispcode commented 1 year ago

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?

YassienW commented 12 months ago

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

chrispcode commented 11 months ago

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

quantizor commented 11 months ago

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.

YassienW commented 11 months ago

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

jguddas commented 11 months ago

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

chrispcode commented 10 months ago

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 commented 10 months ago

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.