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

Custom JSX pragma for better integration with React #3

Open
kripod opened this issue May 17, 2020 · 7 comments
Open

Custom JSX pragma for better integration with React #3

kripod opened this issue May 17, 2020 · 7 comments
Labels
enhancement New feature or request

Comments

@kripod
Copy link
Owner

kripod commented May 17, 2020

Motivation

Inspired by Emotion, a css prop could be attached to JSX elements for better composition possibilities.

Basic example

<p css={{ color: "blue" }}>I am blue</p>

Details

For compatibility purposes, the React Classic Runtime of Babel should be supported.

@kripod kripod added the enhancement New feature or request label May 17, 2020
@kripod
Copy link
Owner Author

kripod commented May 18, 2020

Unlike in most existing solutions, the css prop should be passed between components for atomic composability.

@willhoney7
Copy link

I have mixed opinions on the css prop. I started using it a few years ago in our codebase and have really loved the convenience. It's only now as I step back I'm beginning to realize that maybe it was a mistake? My tweet above mentions two main reasons.

Since it's a pseudo-prop, you can't use it like a real prop (forward it, add to it, etc)

You mention passing the css prop around, and I do think that helps. Another alternative mentioned by Sunil Pai's is to only allow the css prop on dom elements. This makes needing to pass the css prop unnecessary. I think one of these approaches is necessary.

Also, as a developer using this (or any css prop library), to support css and className properly, you'll have to forward both in each of your components. Sounds painful.

Your styles are now defined via three props (className/style/css).

This is the bigger issue to me. I think it's too much of a mental disconnect using both the css and className prop. On twitter, @siddharthkp phrased it well: "css and className both are for base styles. it feels odd to use both in a project."

I like to use a css toolkit (like tailwind, basscss) and then add "custom" css as needed with a css prop. I think if you're using any css classes, you shouldn't use the css prop. They don't compose as well and feel too disconnected. Instead, use classnames (or make your own class-composition-helper).

@amannn
Copy link

amannn commented May 29, 2020

Thank you for this interesting library! Personally I'm largely in favour of the className={css(…)} API if there are no performance downsides.

I recently commented on the emotion repository about this.

The relevant parts are:

However I think it's still a bit confusing that you assign a css prop on an element but then receive className in the component itself. I think another advantage of the className prop API is that it works for edge cases like wrapperClassName without needing additional API like ClassNames. I guess TypeScript integration would also be simpler without this mapping, e.g. a mandatory className prop doesn't seem to work with the current typings (CodeSandbox demo).

For me personally it comes down to:

  • css prop: Convenient API, but slightly magical and some edge cases to consider.
  • css function provided to className: Slightly more verbose, but explicit.

@kripod
Copy link
Owner Author

kripod commented May 29, 2020

Thank you for these detailed thoughts. The css() API would be great if composition can be solved.

For example, when component A passes padding: 16 to component B which already has padding: 0 set by default through className, how may the precedences be managed? I’m thinking about making css() multi-parametered and applying the className-ified rules in order.

A Map could be used for tracking the CSS rule associated with each class name. Pulling this off correctly is complicated, though.

@amannn
Copy link

amannn commented May 29, 2020

So you mean something like this?

function A() {
  return <B className={css({padding: 16})} />;
}

function B({className}) {
  return <p className={css({padding: 0}, className)}>B</p>;
}

Now the css function from B receives the object and _10rtstj. As you mentioned a reverse lookup of the class name within a map to the property that produced the rule could help here.

I guess handling shorthands is something that makes this a bit tricky, e.g.:

css(
  {paddingLeft: 0},
  css({paddingTop: 5}),
  css(
    {padding: 10},
    css({paddingBottom: 20})
  )
);

// Result: `padding: 5px 10px 10px 0`;

This is of course an extreme case, but API-wise this would make sense to me 🙂.

@kripod
Copy link
Owner Author

kripod commented May 29, 2020

Shorthands are hard to handle. I’m not quite sure about how they should be managed. Composition feels a bit unnatural for atomic CSS. Your idea sounds awesome, but it seems to be very hard to implement.

@kripod
Copy link
Owner Author

kripod commented Aug 7, 2020

It seems this could be done with a Babel macro. As for ideas about resolving composition, please see issue #47.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants